diff --git a/PLAN.md b/PLAN.md index b3de04f..b3549da 100644 --- a/PLAN.md +++ b/PLAN.md @@ -20,7 +20,7 @@ The rewrite preserves the intended user-facing behavior, but the LLM-facing `pro 4. Inter-extension communication uses `pi.events` exclusively. UI extensions never import `ProcessManager` or `getManager()`. 5. Query/response uses synchronous callback payloads on named event channels. 6. All `pi.events` listeners are tracked and explicitly unsubscribed on `session_shutdown` to prevent leaks (the EventBus is never cleared by Pi). -7. Config and keybindings live in the core extension, not in `src/`. +7. Config lives in the core extension, not in `src/`. Keybindings are managed by Pi's built-in KeybindingsManager. --- @@ -30,7 +30,7 @@ The rewrite preserves the intended user-facing behavior, but the LLM-facing `pro Phase 1 (src/) --> Phase 2A (minimal core tool) --> Phase 2B (extension notifications) ~~> Phase 2C (output tool) ~~ DONE ~~> Phase 2E (event protocol) ~~ DONE - --> Phase 2F (i18n bridge) + --> Phase 2F (settings, background blocker, i18n bridge) ~~ DONE --> Phase 3 (list) --> Phase 4 (logs) --> Phase 5 (dock) @@ -105,14 +105,28 @@ Implemented and validated in Phase 2E: Latest validation: - `pnpm lint` passes. - `pnpm typecheck` passes. -- `pnpm test` passes with 217 tests. +- `pnpm test` passes with 234 tests. + +Phase 2F settings, background blocker, and i18n bridge is complete. + +Implemented and validated in Phase 2F: +- Config loader using `@aliou/pi-utils-settings` `ConfigLoader` with global/local/memory scopes. +- Config types: `ProcessConfig` (user-facing, all optional) and `ResolvedProcessConfig` (internal, all required with defaults). +- `/ps:settings` command via `registerSettingsCommand` with sectioned settings UI. +- `buildSections` produces Execution and Interception sections (more sections added in Phases 3-5). +- `applySettingChange` converts display values ("on"/"off", numbers, enums) to storage types. +- `REQUEST_CONFIG` handler returns loaded config instead of `{}`. +- Background blocker registered on `pi.on("tool_call")` when `interception.blockBackgroundCommands` is enabled. +- Blocker uses `@aliou/sh` to parse commands into an AST and walks SimpleCommand nodes to detect `&` (Statement.background), `nohup`, `disown`, `setsid` as actual command names (not arguments). Falls back to trailing-`&` regex on parse errors. +- Cleanup is handled by Pi's `session_shutdown` event, not manual `process.once` exit handlers. +- i18n bridge with `createTranslator(overrides?)` and English fallbacks for status, list, stop, and blocker copy. +- Protocol payloads remain structured and language-neutral; localized text is display-only. +- Unit tests for config, build-sections, apply-setting-change, background-blocker, and i18n. Current intentional gaps: -- No settings/config loader yet. - Notification scenario/manual coverage is still pending. -- No background command blocker yet. -- No process-exit/SIGINT/SIGTERM manager registry yet. - No list/logs/dock UI extensions yet. +- Keybindings are managed by Pi's built-in KeybindingsManager; not in extension config. - `logs`, `clear`, and `write` tool actions are deferred. The agent can `read` log file paths returned by `list` or `output` for full-log access. - `package.json` still references `./skills/pi-processes`, but the local `skills/` directory is absent. Either restore the skill later or remove the `pi.skills`/`files` entries during cleanup. - `debug-preview` is intentionally removed from the plan. @@ -139,8 +153,16 @@ Current implemented extension structure: extensions/ processes/ index.ts - hooks/ - cleanup.ts + config/ + index.ts + types.ts + defaults.ts + loader.ts + i18n/ + index.ts + messages.ts + translator.ts + notification-sender.ts tools/ index.ts schema.ts @@ -162,6 +184,45 @@ extensions/ output/ index.ts render.ts + notify.test.ts + notify.ts + utils/ + truncate.ts + hooks/ + cleanup.ts + event-bridge.ts + background-blocker.ts + handlers/ + requests.ts + commands.ts + kill-process.ts + subscriptions.ts + message-renderer.ts + notifications/ + classify.ts + classify.test.ts + log-matchers.ts + log-matchers.test.ts + registry.ts + render-content.ts + render-content.test.ts + service.ts + service.test.ts + types.ts + settings/ + index.ts + build-sections.ts + apply-setting-change.ts + constants.ts +``` + index.ts + render.ts + stop/ + index.ts + render.ts + output/ + index.ts + render.ts utils/ truncate.ts @@ -216,8 +277,15 @@ docs/ extensions/ processes/ index.ts - config.ts - i18n.ts + config/ + index.ts + types.ts + defaults.ts + loader.ts + i18n/ + index.ts + messages.ts + translator.ts notification-sender.ts tools/ index.ts @@ -233,7 +301,6 @@ extensions/ write/ hooks/ cleanup.ts - exit.ts process-notifications.ts background-blocker.ts event-bridge.ts @@ -804,52 +871,55 @@ Log subscription protocol: Session shutdown ordering: 1. Mark this extension instance as shutting down so duplicate shutdown events are ignored. -2. Call all disposers to remove pi.events listeners, manager.onEvent listeners, log subscribers, and process-exit registry entries. +2. Call all disposers to remove pi.events listeners, manager.onEvent listeners, and log subscribers. 3. Call `manager.killAll()`. 4. Call `manager.cleanup()`. --- -### Phase 2F: Settings, background blocker, exit hooks, and i18n bridge +### Phase 2F: Settings, background blocker, and i18n bridge This phase can be split further if it grows. #### Settings/config Add: -- `extensions/processes/config.ts` +- `extensions/processes/config/` (types.ts, defaults.ts, loader.ts, index.ts) - `extensions/processes/settings/index.ts` - `extensions/processes/settings/build-sections.ts` - `extensions/processes/settings/apply-setting-change.ts` -Config sections: -- `processList` -- maxVisibleProcesses, maxPreviewLines -- `output` -- defaultTailLines, maxOutputLines +Config sections (Phase 2F): - `execution` -- shellPath -- `widget` -- showStatusWidget, dockDefaultState, dockHeight -- `follow` -- enabledByDefault, autoHideOnFinish -- `keybindings` -- keybinding overrides - `interception` -- blockBackgroundCommands +Config sections added in later phases: +- `processList` (maxVisibleProcesses, maxPreviewLines) -- Phase 3 (list) +- `output` (defaultTailLines, maxOutputLines) -- Phase 4 (logs) +- `widget` (showStatusWidget, dockDefaultState, dockHeight) -- Phase 5 (dock) +- `follow` (enabledByDefault, autoHideOnFinish) -- Phase 4 (logs) + +Note: keybindings are handled by Pi's built-in KeybindingsManager, not by extension config. + #### Background blocker Add `extensions/processes/hooks/background-blocker.ts`. Behavior: - Listen for bash tool calls. -- Detect background shell patterns such as `&`, `nohup`, `disown`, and `setsid`. +- Use `@aliou/sh` to parse commands into an AST. +- Walk SimpleCommand nodes to detect: `Statement.background` (trailing &), or command name matching `nohup`, `disown`, `setsid`. +- This correctly distinguishes `nohup` as a command from `nohup` as an argument (e.g., `echo nohup`). +- Fall back to a trailing-`&` regex when the command cannot be parsed. - Return a blocking reason that tells the agent to use the `process` tool instead. - Register only when `config.interception.blockBackgroundCommands` is enabled. -#### Exit hooks +#### Exit/cleanup -Add `extensions/processes/hooks/exit.ts`. - -Behavior: -- Keep a global set of live extension-owned managers. -- Attach one-time handlers for `exit`, `SIGINT`, and `SIGTERM`. -- On process exit, kill all registered live managers. -- Return an unregister function for session cleanup. +No custom exit hooks needed. Pi handles process lifecycle: +- `session_shutdown` event fires before the extension runtime is torn down. +- Pi registers its own SIGTERM/SIGHUP handlers that trigger graceful shutdown. +- The cleanup hook in `hooks/cleanup.ts` subscribes to `session_shutdown` and kills all live managers. #### i18n bridge @@ -857,7 +927,7 @@ This is inspired by GitHub PR #34, which proposed a small localization bridge fo Recommendation: - Keep i18n out of `src/`. -- Add `extensions/processes/i18n.ts` for Pi-facing text only. +- Add `extensions/processes/i18n/` (messages.ts, translator.ts, index.ts) for Pi-facing text only. - Keep protocol payloads structured and language-neutral. - Renderers/components call a translator function with stable keys and params. - Provide English fallbacks in the package. @@ -1296,7 +1366,7 @@ The initial rewrite already removed the old Pi-aware `src/` tree. At cleanup tim Remove if present: - `src/index.ts` (old entry point) -- `src/config.ts` (moved to `extensions/processes/config.ts`) +- `src/config.ts` (moved to `extensions/processes/config/`) - `src/tools/` (moved to `extensions/processes/tools/`) - `src/hooks/` (moved to `extensions/processes/hooks/`) - `src/commands/` (moved to list/logs/dock extensions) @@ -1523,9 +1593,9 @@ Cross-session persistence is not implemented in Phase 1. `ProcessManager` is a p `get-manager.ts` is a small factory. The core extension owns the returned manager in its closure and must shut it down on `session_shutdown`. -Session shutdown is instance-owned: dispose the current extension instance's listeners, unregister that manager from process-exit cleanup, then call `manager.killAll()` and `manager.cleanup()` on the same manager object. +Session shutdown is instance-owned: dispose the current extension instance's listeners, kill all processes managed by that instance, then call `manager.killAll()` and `manager.cleanup()` on the same manager object. -Actual Node process exit is registry-owned: `hooks/exit.ts` keeps a global `Set` of currently live extension-owned managers and one guarded set of process exit handlers. The handlers iterate the current set and call `killAll()` on each manager. +The cleanup hook subscribes to Pi's `session_shutdown` event. Pi handles SIGTERM/SIGHUP and emits `session_shutdown` before tearing down the extension runtime. No custom `process.once` exit handlers are needed. The skill file and prompt guidelines do not need persistence-specific behavior. @@ -1535,7 +1605,7 @@ The skill file and prompt guidelines do not need persistence-specific behavior. 2. **Stale Pi context**: After `/new`/`/fork`/`/resume`, the old `pi` proxy throws on any method call. Do not solve this by swallowing stale-proxy errors. Prevent stale calls through lifecycle ownership: notification services must be extension-instance scoped, expose `dispose()`, set a disposed flag before cleanup, unsubscribe manager/pi event listeners, and never send after disposal. -3. **Exit hook duplication**: The `process.once("exit", ...)` handler in `hooks/exit.ts` must be guarded by a globalThis flag. Without this, each extension reload would add another exit handler. The guarded handler must iterate a global manager set, not close over one manager from the first extension load. +3. **Exit hook duplication**: No longer relevant. Cleanup is handled by Pi's `session_shutdown` event rather than custom `process.once` handlers. Pi manages its own signal handlers and emits `session_shutdown` before tearing down extensions. 4. **Manager construction options**: `getManager()` creates a new manager for the extension instance. The `getConfiguredShellPath` callback is a closure that reads from `configLoader.getConfig()`, so config changes are picked up automatically without needing to reconstruct the manager. diff --git a/extensions/processes/config/defaults.ts b/extensions/processes/config/defaults.ts new file mode 100644 index 0000000..a81b627 --- /dev/null +++ b/extensions/processes/config/defaults.ts @@ -0,0 +1,10 @@ +import type { ResolvedProcessConfig } from "./types"; + +export const DEFAULT_CONFIG: ResolvedProcessConfig = { + execution: { + shellPath: undefined, + }, + interception: { + blockBackgroundCommands: true, + }, +}; diff --git a/extensions/processes/config/index.ts b/extensions/processes/config/index.ts new file mode 100644 index 0000000..032d5f1 --- /dev/null +++ b/extensions/processes/config/index.ts @@ -0,0 +1,3 @@ +export { DEFAULT_CONFIG } from "./defaults"; +export { configLoader, createSettingsConfigStore } from "./loader"; +export type { ProcessConfig, ResolvedProcessConfig } from "./types"; diff --git a/extensions/processes/config/loader.ts b/extensions/processes/config/loader.ts new file mode 100644 index 0000000..30a7869 --- /dev/null +++ b/extensions/processes/config/loader.ts @@ -0,0 +1,36 @@ +import { + buildSchemaUrl, + ConfigLoader, + type ConfigStore, + type Scope, +} from "@aliou/pi-utils-settings"; +import pkg from "../../../package.json" with { type: "json" }; +import { DEFAULT_CONFIG } from "./defaults"; +import type { ProcessConfig, ResolvedProcessConfig } from "./types"; + +export const configLoader = new ConfigLoader< + ProcessConfig, + ResolvedProcessConfig +>("processes", DEFAULT_CONFIG, { + scopes: ["global", "local", "memory"], + schemaUrl: buildSchemaUrl(pkg.name, pkg.version), +}); + +export function createSettingsConfigStore(): ConfigStore< + ProcessConfig, + ResolvedProcessConfig +> { + return { + save: (scope, config) => configLoader.save(scope, config), + getConfig: () => configLoader.getConfig(), + getRawConfig: (scope) => configLoader.getRawConfig(scope), + hasScope: (scope) => configLoader.hasScope(scope), + hasConfig: (scope) => configLoader.hasConfig(scope), + getEnabledScopes: () => { + const enabled = new Set(configLoader.getEnabledScopes()); + return (["global", "local", "memory"] as Scope[]).filter((scope) => + enabled.has(scope), + ); + }, + }; +} diff --git a/extensions/processes/config/types.ts b/extensions/processes/config/types.ts new file mode 100644 index 0000000..0bf83d4 --- /dev/null +++ b/extensions/processes/config/types.ts @@ -0,0 +1,29 @@ +/** + * Extension config types. + * + * User-facing schema is ProcessConfig (all fields optional). + * Internal resolved schema is ResolvedProcessConfig (all fields required, defaults applied). + */ + +export interface ExecutionConfig { + shellPath?: string; +} + +export interface InterceptionConfig { + blockBackgroundCommands?: boolean; +} + +export interface ProcessConfig { + $schema?: string; + execution?: ExecutionConfig; + interception?: InterceptionConfig; +} + +export interface ResolvedProcessConfig { + execution: { + shellPath: string | undefined; + }; + interception: { + blockBackgroundCommands: boolean; + }; +} diff --git a/extensions/processes/handlers/requests.test.ts b/extensions/processes/handlers/requests.test.ts index 571f2b6..00c73e3 100644 --- a/extensions/processes/handlers/requests.test.ts +++ b/extensions/processes/handlers/requests.test.ts @@ -4,6 +4,8 @@ import { describe, expect, it, vi } from "vitest"; import type { ProcessManager } from "../../../src/manager"; import { CHANNELS } from "../../../src/protocol"; import type { ProcessInfo } from "../../../src/types"; +import type { ResolvedProcessConfig } from "../config"; +import { DEFAULT_CONFIG } from "../config"; import { registerRequestHandlers } from "./requests"; function makeInfo(overrides: Partial = {}): ProcessInfo { @@ -27,6 +29,8 @@ function makeInfo(overrides: Partial = {}): ProcessInfo { }; } +const getConfig = () => DEFAULT_CONFIG; + describe("registerRequestHandlers", () => { it("replies to manager read requests", () => { const events = createEventBus(); @@ -48,7 +52,7 @@ describe("registerRequestHandlers", () => { getFileSize: vi.fn(() => ({ stdout: 1, stderr: 2 })), } as unknown as ProcessManager; - registerRequestHandlers(events, manager); + registerRequestHandlers(events, manager, getConfig); const listReply = vi.fn(); events.emit(CHANNELS.REQUEST_LIST, { reply: listReply }); @@ -101,15 +105,19 @@ describe("registerRequestHandlers", () => { expect(sizeReply).toHaveBeenCalledWith({ stdout: 1, stderr: 2 }); }); - it("replies with temporary empty config", () => { + it("replies with loaded config", () => { const events = createEventBus(); const manager = {} as ProcessManager; + const config: ResolvedProcessConfig = { + ...DEFAULT_CONFIG, + execution: { shellPath: "/bin/bash" }, + }; const reply = vi.fn(); - registerRequestHandlers(events, manager); + registerRequestHandlers(events, manager, () => config); events.emit(CHANNELS.REQUEST_CONFIG, { reply }); - expect(reply).toHaveBeenCalledWith({}); + expect(reply).toHaveBeenCalledWith(config); }); it("ignores malformed request payloads", () => { @@ -123,7 +131,7 @@ describe("registerRequestHandlers", () => { getFileSize: vi.fn(() => null), } as unknown as ProcessManager; - registerRequestHandlers(events, manager); + registerRequestHandlers(events, manager, getConfig); events.emit(CHANNELS.REQUEST_LIST, null); events.emit(CHANNELS.REQUEST_LIST, {}); events.emit(CHANNELS.REQUEST_GET, { id: 123, reply: vi.fn() }); @@ -154,7 +162,7 @@ describe("registerRequestHandlers", () => { const manager = { list: vi.fn(() => []) } as unknown as ProcessManager; const reply = vi.fn(); - const dispose = registerRequestHandlers(events, manager); + const dispose = registerRequestHandlers(events, manager, getConfig); dispose(); events.emit(CHANNELS.REQUEST_LIST, { reply }); diff --git a/extensions/processes/handlers/requests.ts b/extensions/processes/handlers/requests.ts index 8b22b5a..26a2383 100644 --- a/extensions/processes/handlers/requests.ts +++ b/extensions/processes/handlers/requests.ts @@ -1,5 +1,4 @@ import type { EventBus } from "@earendil-works/pi-coding-agent"; - import type { ProcessManager } from "../../../src/manager"; import { CHANNELS, @@ -11,10 +10,12 @@ import { type RequestLogFilesPayload, type RequestOutputPayload, } from "../../../src/protocol"; +import type { ResolvedProcessConfig } from "../config"; export function registerRequestHandlers( events: EventBus, manager: ProcessManager, + getConfig: () => ResolvedProcessConfig, ): () => void { const disposers = [ events.on(CHANNELS.REQUEST_LIST, (payload) => { @@ -57,8 +58,7 @@ export function registerRequestHandlers( const request = payload as RequestConfigPayload; if (!isRequestConfigPayload(request)) return; - // TODO(Phase 2F): return loaded process settings. - request.reply({}); + request.reply(getConfig()); }), ]; diff --git a/extensions/processes/hooks/background-blocker.test.ts b/extensions/processes/hooks/background-blocker.test.ts new file mode 100644 index 0000000..85edcd2 --- /dev/null +++ b/extensions/processes/hooks/background-blocker.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from "vitest"; + +import { isBackgroundCommand } from "./background-blocker"; + +describe("isBackgroundCommand", () => { + it("blocks trailing ampersand", () => { + expect(isBackgroundCommand("sleep 10 &")).toBe(true); + expect(isBackgroundCommand("server &")).toBe(true); + }); + + it("blocks & at end of command with no trailing space", () => { + expect(isBackgroundCommand("sleep 10&")).toBe(true); + }); + + it("blocks nohup as first command", () => { + expect(isBackgroundCommand("nohup ./server.sh")).toBe(true); + expect(isBackgroundCommand("nohup node app.js &")).toBe(true); + expect(isBackgroundCommand("nohup")).toBe(true); + }); + + it("blocks nohup after shell operators", () => { + expect(isBackgroundCommand("sleep 10; nohup ./server.sh")).toBe(true); + expect(isBackgroundCommand("ls | nohup cat")).toBe(true); + expect(isBackgroundCommand("true && nohup ./server.sh")).toBe(true); + expect(isBackgroundCommand("build || nohup ./fallback.sh")).toBe(true); + }); + + it("blocks disown as first command or after operators", () => { + expect(isBackgroundCommand("disown %1")).toBe(true); + expect(isBackgroundCommand("disown -h %2")).toBe(true); + expect(isBackgroundCommand("disown")).toBe(true); + expect(isBackgroundCommand("true && disown")).toBe(true); + }); + + it("blocks setsid as first command or after operators", () => { + expect(isBackgroundCommand("setsid ./daemon")).toBe(true); + expect(isBackgroundCommand("setsid node watcher.js")).toBe(true); + expect(isBackgroundCommand("setsid")).toBe(true); + expect(isBackgroundCommand("true || setsid ./daemon")).toBe(true); + }); + + it("allows regular commands", () => { + expect(isBackgroundCommand("echo hello")).toBe(false); + expect(isBackgroundCommand("pnpm dev")).toBe(false); + expect(isBackgroundCommand("ls -la")).toBe(false); + expect(isBackgroundCommand("git status")).toBe(false); + }); + + it("allows logical AND (&&)", () => { + expect(isBackgroundCommand("pnpm build && pnpm test")).toBe(false); + expect(isBackgroundCommand("cd /tmp && ls")).toBe(false); + }); + + it("does not block nohup/disown/setsid as arguments to other commands", () => { + expect(isBackgroundCommand("echo nohup")).toBe(false); + expect(isBackgroundCommand("echo disown")).toBe(false); + expect(isBackgroundCommand("echo setsid")).toBe(false); + }); + + it("does not block keywords as substring of file paths", () => { + expect(isBackgroundCommand("cat /path/nohup.log")).toBe(false); + }); + + it("handles empty and whitespace-only commands", () => { + expect(isBackgroundCommand("")).toBe(false); + expect(isBackgroundCommand(" ")).toBe(false); + }); + + it("falls back to regex for malformed shell commands", () => { + // This is syntactically invalid; the parser will throw and we + // fall back to a trailing & regex. + expect(isBackgroundCommand("something &")).toBe(true); + }); +}); diff --git a/extensions/processes/hooks/background-blocker.ts b/extensions/processes/hooks/background-blocker.ts new file mode 100644 index 0000000..ee2d4ac --- /dev/null +++ b/extensions/processes/hooks/background-blocker.ts @@ -0,0 +1,201 @@ +/** + * Background command blocker. + * + * Listens for bash tool calls and blocks commands that contain + * shell background patterns (trailing &, nohup, disown, setsid). + * + * Uses @aliou/sh to parse commands into an AST and walk all + * SimpleCommand nodes, checking for: + * - Statement.background (trailing &) + * - Command name matching nohup/disown/setsid + * + * Registered only when config.interception.blockBackgroundCommands is enabled. + * Returns a blocking reason that tells the agent to use the process tool instead. + */ + +import type { + Command, + Program, + SimpleCommand, + Statement, + Word, +} from "@aliou/sh"; +import { parse } from "@aliou/sh"; +import type { + BashToolCallEvent, + ExtensionAPI, + ToolCallEventResult, +} from "@earendil-works/pi-coding-agent"; + +import { t } from "../i18n"; + +const BACKGROUND_KEYWORDS = new Set(["nohup", "disown", "setsid"]); + +/** + * Extract the literal command name from a Word node. + * Returns the value of the first Literal part, or undefined if + * the word starts with a non-literal (e.g., variable expansion). + */ +function getCommandName(word: Word): string | undefined { + const firstPart = word.parts?.[0]; + if (firstPart?.type === "Literal") { + return firstPart.value; + } + return undefined; +} + +/** + * Check if a SimpleCommand's name matches a background keyword. + */ +function isBackgroundSimpleCommand(cmd: SimpleCommand): boolean { + const firstName = cmd.words?.[0] ? getCommandName(cmd.words[0]) : undefined; + return firstName !== undefined && BACKGROUND_KEYWORDS.has(firstName); +} + +/** + * Recursively walk a Command AST, returning true if any branch + * contains a background pattern (trailing & or background keyword). + */ +function walkCommand(command: Command, isBg: { value: boolean }): void { + switch (command.type) { + case "SimpleCommand": + if (isBackgroundSimpleCommand(command)) { + isBg.value = true; + } + break; + + case "Pipeline": + for (const stmt of command.commands ?? []) { + walkStatement(stmt, isBg); + if (isBg.value) return; + } + break; + + case "Logical": + walkStatement(command.left, isBg); + if (isBg.value) return; + walkStatement(command.right, isBg); + break; + + case "Subshell": + case "Block": + for (const stmt of command.body ?? []) { + walkStatement(stmt, isBg); + if (isBg.value) return; + } + break; + + case "IfClause": + for (const stmt of command.cond ?? []) { + walkStatement(stmt, isBg); + if (isBg.value) return; + } + for (const stmt of command.then ?? []) { + walkStatement(stmt, isBg); + if (isBg.value) return; + } + for (const stmt of command.else ?? []) { + walkStatement(stmt, isBg); + if (isBg.value) return; + } + break; + + case "WhileClause": + case "ForClause": + case "SelectClause": + case "CStyleLoop": + for (const stmt of command.body ?? []) { + walkStatement(stmt, isBg); + if (isBg.value) return; + } + break; + + case "FunctionDecl": + for (const stmt of command.body ?? []) { + walkStatement(stmt, isBg); + if (isBg.value) return; + } + break; + + case "CaseClause": + for (const item of command.items ?? []) { + for (const stmt of item.body ?? []) { + walkStatement(stmt, isBg); + if (isBg.value) return; + } + } + break; + + default: + // TimeClause, TestClause, ArithCmd, CoprocClause, DeclClause, LetClause: + // these don't carry SimpleCommands that are background indicators. + break; + } +} + +function walkStatement(stmt: Statement, isBg: { value: boolean }): void { + if (stmt.background) { + isBg.value = true; + return; + } + if (stmt.command) { + walkCommand(stmt.command, isBg); + } +} + +/** + * Check if a command string contains background execution patterns. + * Returns true if the command should be blocked. + * + * Uses @aliou/sh to parse the command into an AST. If the command + * cannot be parsed (syntax error), falls back to a simple regex + * heuristic for trailing &. + */ +export function isBackgroundCommand(command: string): boolean { + let program: Program; + + try { + const result = parse(command); + program = result.ast; + } catch { + // Fallback: if the command can't be parsed (syntax error), + // use a simple regex heuristic for trailing &. + return /\s*&\s*$/.test(command); + } + + const isBg = { value: false }; + + for (const stmt of program.body ?? []) { + walkStatement(stmt, isBg); + if (isBg.value) return true; + } + + return false; +} + +/** + * Register the background command blocker on the tool_call event. + * + * The blocker only activates when the provided isEnabled callback returns true. + * pi.on() does not return a disposer, so cleanup is handled by session_shutdown. + */ +export function registerBackgroundBlocker( + pi: ExtensionAPI, + isEnabled: () => boolean, +): void { + pi.on("tool_call", (event) => { + if (!isEnabled()) return; + + if (event.toolName !== "bash") return; + + const bashEvent = event as BashToolCallEvent; + const command = bashEvent.input.command; + + if (isBackgroundCommand(command)) { + return { + block: true, + reason: t("process.blocker.background_command"), + } satisfies ToolCallEventResult; + } + }); +} diff --git a/extensions/processes/i18n/index.ts b/extensions/processes/i18n/index.ts new file mode 100644 index 0000000..33295ad --- /dev/null +++ b/extensions/processes/i18n/index.ts @@ -0,0 +1,3 @@ +export type { MessageKey } from "./messages"; +export type { Translator } from "./translator"; +export { createTranslator, t } from "./translator"; diff --git a/extensions/processes/i18n/messages.ts b/extensions/processes/i18n/messages.ts new file mode 100644 index 0000000..93fe5b1 --- /dev/null +++ b/extensions/processes/i18n/messages.ts @@ -0,0 +1,27 @@ +export type MessageKey = + | "process.list.empty" + | "process.list.summary" + | "process.stop.not_found" + | "process.stop.timeout" + | "process.status.running" + | "process.status.terminating" + | "process.status.terminate_timeout" + | "process.status.exited" + | "process.status.killed" + | "process.status.failed" + | "process.blocker.background_command"; + +export const ENGLISH: Record = { + "process.list.empty": "No running processes", + "process.list.summary": "{count} process{count_plural}", + "process.stop.not_found": "Process not found: {id}", + "process.stop.timeout": "Process kill timed out: {id}", + "process.status.running": "running", + "process.status.terminating": "terminating", + "process.status.terminate_timeout": "terminate timeout", + "process.status.exited": "exited ({exitCode})", + "process.status.killed": "killed", + "process.status.failed": "failed ({errorMessage})", + "process.blocker.background_command": + "Shell background patterns (&, nohup, disown, setsid) are not allowed. Use the process tool to manage background commands.", +}; diff --git a/extensions/processes/i18n/translator.ts b/extensions/processes/i18n/translator.ts new file mode 100644 index 0000000..b7f8113 --- /dev/null +++ b/extensions/processes/i18n/translator.ts @@ -0,0 +1,49 @@ +import type { MessageKey } from "./messages"; +import { ENGLISH } from "./messages"; + +export type { MessageKey } from "./messages"; + +export type Translator = ( + key: MessageKey, + params?: Record, +) => string; + +function pluralize(count: number): string { + return count === 1 ? "" : "es"; +} + +function formatTemplate( + template: string, + params: Record, +): string { + return template.replace(/\{(\w+)\}/g, (match, key) => { + if (key === "count_plural") { + const count = Number(params.count) || 0; + return pluralize(count); + } + return String(params[key] ?? match); + }); +} + +/** + * Create a translator with optional overrides. + * + * Overrides map message keys to custom strings. Unoverridden keys + * fall back to English defaults. + */ +export function createTranslator( + overrides?: Partial>, +): Translator { + const strings = { ...ENGLISH, ...overrides }; + + return (key, params) => { + const template = strings[key] ?? ENGLISH[key] ?? key; + if (!params) return template; + return formatTemplate(template, params); + }; +} + +/** + * Default translator with English fallbacks. No overrides. + */ +export const t: Translator = createTranslator(); diff --git a/extensions/processes/index.ts b/extensions/processes/index.ts index c54401c..b39b2d6 100644 --- a/extensions/processes/index.ts +++ b/extensions/processes/index.ts @@ -1,9 +1,10 @@ import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; - import { getManager } from "../../src/get-manager"; +import { configLoader } from "./config"; import { registerCommandHandlers } from "./handlers/commands"; import { registerRequestHandlers } from "./handlers/requests"; import { registerLogSubscriptions } from "./handlers/subscriptions"; +import { registerBackgroundBlocker } from "./hooks/background-blocker"; import { registerCleanupHook } from "./hooks/cleanup"; import { registerEventBridge } from "./hooks/event-bridge"; import { registerProcessNotificationRenderer } from "./message-renderer"; @@ -11,10 +12,25 @@ import { createNotificationRegistry, createNotificationService, } from "./notifications/service"; +import { registerProcessSettings } from "./settings"; import { registerProcessTool } from "./tools"; -export default function processesExtension(pi: ExtensionAPI): void { - const manager = getManager(); +export default async function processesExtension( + pi: ExtensionAPI, +): Promise { + // Load config. If the config file is malformed, fall back to defaults + // rather than preventing the extension from initialising. + try { + await configLoader.load(); + } catch { + // ConfigLoader.load() throws on unreadable files; defaults are still + // available via getConfig() after a failed load. + void 0; + } + + const manager = getManager({ + getConfiguredShellPath: () => configLoader.getConfig().execution.shellPath, + }); const notifications = createNotificationRegistry(); const notificationService = createNotificationService({ pi, @@ -23,15 +39,24 @@ export default function processesExtension(pi: ExtensionAPI): void { getProcess: (id) => manager.get(id), }); + const getConfig = () => configLoader.getConfig(); + const disposers = [ registerEventBridge(pi.events, manager), - registerRequestHandlers(pi.events, manager), + registerRequestHandlers(pi.events, manager, getConfig), registerCommandHandlers(pi.events, manager, notifications), registerLogSubscriptions(pi.events, manager), ]; + registerBackgroundBlocker( + pi, + () => getConfig().interception.blockBackgroundCommands, + ); + registerProcessNotificationRenderer(pi); registerProcessTool(pi, manager, notifications); + registerProcessSettings(pi); + registerCleanupHook(pi, { manager, notifications, diff --git a/extensions/processes/settings/apply-setting-change.test.ts b/extensions/processes/settings/apply-setting-change.test.ts new file mode 100644 index 0000000..aef34ae --- /dev/null +++ b/extensions/processes/settings/apply-setting-change.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import type { ProcessConfig } from "../config"; +import { applySettingChange } from "./apply-setting-change"; + +describe("applySettingChange", () => { + const base: ProcessConfig = {}; + + it("converts 'on'/'off' to boolean", () => { + const on = applySettingChange( + "interception.blockBackgroundCommands", + "on", + base, + ); + expect(on?.interception?.blockBackgroundCommands).toBe(true); + + const off = applySettingChange( + "interception.blockBackgroundCommands", + "off", + base, + ); + expect(off?.interception?.blockBackgroundCommands).toBe(false); + }); + + it("converts '(default)' sentinel to empty string for shellPath", () => { + const result = applySettingChange("execution.shellPath", "(default)", { + execution: { shellPath: "/bin/zsh" }, + }); + expect(result?.execution?.shellPath).toBe(""); + }); + + it("returns null for unknown setting IDs", () => { + expect(applySettingChange("unknown.field", "value", base)).toBeNull(); + }); +}); diff --git a/extensions/processes/settings/apply-setting-change.ts b/extensions/processes/settings/apply-setting-change.ts new file mode 100644 index 0000000..56667b2 --- /dev/null +++ b/extensions/processes/settings/apply-setting-change.ts @@ -0,0 +1,35 @@ +/** + * Apply a setting change to the config. + * + * Converts display values (e.g. "on"/"off") to storage types (booleans, numbers). + * Returns the updated config, or null to fall through to default string storage. + */ + +import { setNestedValue } from "@aliou/pi-utils-settings"; + +import type { ProcessConfig } from "../config"; + +const BOOLEAN_FIELDS = new Set(["interception.blockBackgroundCommands"]); + +const TEXT_FIELDS = new Set(["execution.shellPath"]); + +export function applySettingChange( + id: string, + newValue: string, + config: ProcessConfig, +): ProcessConfig | null { + const updated = structuredClone(config); + + if (BOOLEAN_FIELDS.has(id)) { + setNestedValue(updated, id, newValue === "on"); + return updated; + } + + if (TEXT_FIELDS.has(id)) { + setNestedValue(updated, id, newValue === "(default)" ? "" : newValue); + return updated; + } + + // Unknown field: fall through to default string storage + return null; +} diff --git a/extensions/processes/settings/build-sections.test.ts b/extensions/processes/settings/build-sections.test.ts new file mode 100644 index 0000000..8928e5b --- /dev/null +++ b/extensions/processes/settings/build-sections.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import type { ProcessConfig } from "../config"; +import { DEFAULT_CONFIG } from "../config"; +import { buildSections } from "./build-sections"; + +describe("buildSections", () => { + const resolved = { ...DEFAULT_CONFIG }; + const ctx = { + setDraft: () => {}, + scope: "global" as const, + isInherited: (_path: string) => false, + }; + + it("produces all expected section labels", () => { + const sections = buildSections(null, resolved, ctx); + const labels = sections.map((s) => s.label); + expect(labels).toEqual(["Execution", "Interception"]); + }); + + it("distinguishes scoped vs inherited values", () => { + const scoped: ProcessConfig = { + interception: { blockBackgroundCommands: false }, + }; + const sections = buildSections(scoped, resolved, ctx); + + const inherited = buildSections(null, resolved, ctx); + const getBlockerValue = (s: typeof sections) => + s + .find((s) => s.label === "Interception") + ?.items.find((i) => i.id === "interception.blockBackgroundCommands") + ?.currentValue; + + expect(getBlockerValue(sections)).toBe("off"); + expect(getBlockerValue(inherited)).toBe("inherited: on"); + }); +}); diff --git a/extensions/processes/settings/build-sections.ts b/extensions/processes/settings/build-sections.ts new file mode 100644 index 0000000..dfde172 --- /dev/null +++ b/extensions/processes/settings/build-sections.ts @@ -0,0 +1,98 @@ +/** + * Build settings sections for the processes extension. + * + * Each section maps to a top-level config group. + * Values show the scope-local override or the inherited resolved value. + */ + +import type { Scope, SettingsSection } from "@aliou/pi-utils-settings"; +import type { SettingItem } from "@earendil-works/pi-tui"; + +import type { ProcessConfig, ResolvedProcessConfig } from "../config"; + +interface BuildSectionsContext { + setDraft: (config: ProcessConfig) => void; + scope: Scope; + isInherited: (path: string) => boolean; +} + +export function buildSections( + tabConfig: ProcessConfig | null, + resolved: ResolvedProcessConfig, + _ctx: BuildSectionsContext, +): SettingsSection[] { + const scopedConfig = structuredClone(tabConfig ?? {}) as ProcessConfig; + + function boolItem( + id: string, + label: string, + description: string, + scopedValue: boolean | undefined, + resolvedValue: boolean, + ): SettingItem { + const display = + scopedValue === undefined + ? `inherited: ${resolvedValue ? "on" : "off"}` + : scopedValue + ? "on" + : "off"; + return { + id, + label, + description, + currentValue: display, + values: ["on", "off"], + }; + } + + function textItem( + id: string, + label: string, + description: string, + scopedValue: string | undefined, + resolvedValue: string | undefined, + emptyText: string, + ): SettingItem { + const display = + scopedValue === undefined + ? resolvedValue === undefined + ? emptyText + : `inherited: ${resolvedValue || emptyText}` + : scopedValue || emptyText; + return { + id, + label, + description, + currentValue: display, + }; + } + + const executionSection: SettingsSection = { + label: "Execution", + items: [ + textItem( + "execution.shellPath", + "Shell path", + "Path to the shell executable for process commands. Leave empty to use the system default.", + scopedConfig.execution?.shellPath, + resolved.execution.shellPath, + "(default)", + ), + ], + }; + + const interceptionSection: SettingsSection = { + label: "Interception", + items: [ + boolItem( + "interception.blockBackgroundCommands", + "Block background commands", + "Block shell background patterns (&, nohup, disown, setsid) in bash tool calls. Redirects to the process tool instead.", + scopedConfig.interception?.blockBackgroundCommands, + resolved.interception.blockBackgroundCommands, + ), + ], + }; + + return [executionSection, interceptionSection]; +} diff --git a/extensions/processes/settings/index.ts b/extensions/processes/settings/index.ts new file mode 100644 index 0000000..cb36ce2 --- /dev/null +++ b/extensions/processes/settings/index.ts @@ -0,0 +1,41 @@ +/** + * Settings registration for the processes extension. + * + * Uses @aliou/pi-utils-settings infrastructure for a /ps:settings command + * with Global/Local/Memory tabs and sectioned settings. + */ + +import { + registerSettingsCommand, + type SettingsSection, +} from "@aliou/pi-utils-settings"; +import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; + +import type { ProcessConfig, ResolvedProcessConfig } from "../config"; +import { createSettingsConfigStore } from "../config"; +import { applySettingChange } from "./apply-setting-change"; +import { buildSections } from "./build-sections"; + +export function registerProcessSettings(pi: ExtensionAPI): void { + const configStore = createSettingsConfigStore(); + + registerSettingsCommand(pi, { + commandName: "ps:settings", + title: "Processes Settings", + configStore, + buildSections: ( + tabConfig: ProcessConfig | null, + resolved: ResolvedProcessConfig, + ctx, + ): SettingsSection[] => { + return buildSections(tabConfig, resolved, { + setDraft: ctx.setDraft, + scope: ctx.scope, + isInherited: ctx.isInherited, + }); + }, + onSettingChange: (id, newValue, config) => { + return applySettingChange(id, newValue, config); + }, + }); +} diff --git a/package.json b/package.json index 5108ffc..6c92270 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "CONTRIBUTING.md" ], "dependencies": { - "@aliou/pi-utils-settings": "^0.15.1", + "@aliou/pi-utils-settings": "^0.16.0", "@aliou/pi-utils-ui": "^0.4.1", "@aliou/sh": "^0.1.0", "typebox": "^1.0.0" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ef9a7ec..f1377f6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -13,8 +13,8 @@ importers: .: dependencies: '@aliou/pi-utils-settings': - specifier: ^0.15.1 - version: 0.15.1(@earendil-works/pi-coding-agent@0.74.0(ws@8.19.0)(zod@3.25.76))(@earendil-works/pi-tui@0.74.0) + specifier: ^0.16.0 + version: 0.16.0(@earendil-works/pi-coding-agent@0.74.0(ws@8.19.0)(zod@3.25.76))(@earendil-works/pi-tui@0.74.0) '@aliou/pi-utils-ui': specifier: ^0.4.1 version: 0.4.1(@earendil-works/pi-coding-agent@0.74.0(ws@8.19.0)(zod@3.25.76))(@earendil-works/pi-tui@0.74.0) @@ -69,8 +69,8 @@ packages: peerDependencies: '@biomejs/biome': '>=2.4.0' - '@aliou/pi-utils-settings@0.15.1': - resolution: {integrity: sha512-oECJ4c/BaYQvzMKHVuNg2HJOd9Fh4+mWglKOqeDfu8mu28VYpJqMrAOqgOh3XWPTb2cPWzlavPEjFZ2FzUBBQg==} + '@aliou/pi-utils-settings@0.16.0': + resolution: {integrity: sha512-mVhwtt7dKMJKK/QM66Gx2tJlN2CfLB7DXDE4ClWVT2DXENmiRyO2ptb0uY7pTJod+57CdfpGdq2NXpoaDrf2Ww==} peerDependencies: '@earendil-works/pi-coding-agent': '>=0.74.0 <1' peerDependenciesMeta: @@ -2250,7 +2250,7 @@ snapshots: dependencies: '@biomejs/biome': 2.4.15 - '@aliou/pi-utils-settings@0.15.1(@earendil-works/pi-coding-agent@0.74.0(ws@8.19.0)(zod@3.25.76))(@earendil-works/pi-tui@0.74.0)': + '@aliou/pi-utils-settings@0.16.0(@earendil-works/pi-coding-agent@0.74.0(ws@8.19.0)(zod@3.25.76))(@earendil-works/pi-tui@0.74.0)': dependencies: '@aliou/pi-utils-ui': 0.4.1(@earendil-works/pi-coding-agent@0.74.0(ws@8.19.0)(zod@3.25.76))(@earendil-works/pi-tui@0.74.0) optionalDependencies: