diff --git a/.changeset/intentional-stop-context-notification.md b/.changeset/intentional-stop-context-notification.md new file mode 100644 index 0000000..afb3e59 --- /dev/null +++ b/.changeset/intentional-stop-context-notification.md @@ -0,0 +1,7 @@ +--- +"@aliou/pi-processes": patch +--- + +Intentional process kills via /ps are now delivered as a context-level +notification instead of being fully suppressed. The agent is not woken, +but it receives the lifecycle message so it can keep its state accurate. diff --git a/AGENTS.md b/AGENTS.md index e8b56f8..66e365b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,8 +60,13 @@ Avoid fixed sleeps in both unit and e2e tests. Prefer event-driven helpers that - `plugins/` - repo-local Biome GritQL lint plugins, registered in `biome.json` - `skills/` - shipped package skills consumed by Pi - `.agents/skills/` - local repo-only skills for development workflows +- `docs/` - documentation and design notes (current behavior, architecture, canonical references, future design proposals). See `docs/README.md` for the index. -Future design notes live in `docs/future-persistent-manager.md` and `docs/future-cleanup-hooks.md`. Keep implemented behavior in living docs and put new active plans under `.agents/plans/`. +## Documentation + +`docs/` holds both living docs (current behavior) and future-design notes (proposed work, prefixed `future-`). When a change to lifecycle, notifications, config, or public behavior lands, update the relevant `docs/` entry (start with `docs/notifications.md` for notification/event flow) and retire the matching future-design note by folding its landed behavior into a living doc. + +When adding a doc, name it for its subject (`docs/notifications.md`, not `docs/new-foo.md`) and add it to `docs/README.md`. Anchor call-stack nodes to real symbols and plain repo paths, never line numbers. ## Rendering conventions diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 0000000..57400dc --- /dev/null +++ b/docs/README.md @@ -0,0 +1,13 @@ +# docs + +Evergreen documentation and design notes for `pi-processes`. Current behavior and architecture live here; proposed or unsettled work is marked in the file itself. + +## Index + +- `notifications.md` — how process lifecycle and log-watch events become agent notifications: the two-layer fanout, per-process notify config and defaults, attention-level mapping, forced display for crashes/failures, the intentional-stop config bypass, the `terminate_timeout` non-emit path, and log-match matching and rate limiting. Includes per-end-state call stacks and a summary matrix. +- `future-persistent-manager.md` — proposed design for keeping processes alive across `/reload`, `/new`, and `/fork`. +- `future-cleanup-hooks.md` — proposed design for generic cleanup hooks run when a managed process stops. + +## Upkeep + +Update a doc when the flow, config, or attention rules it describes change. Anchor call-stack nodes to real symbols and plain repo paths, never line numbers. A stale call graph misleads worse than none — delete or fix it when the code drifts. diff --git a/docs/notifications.md b/docs/notifications.md new file mode 100644 index 0000000..a9b7873 --- /dev/null +++ b/docs/notifications.md @@ -0,0 +1,323 @@ +# Process notifications + +This document describes how process lifecycle and log-watch events become +notifications that reach the agent, which user options control each path, and +which paths bypass user config. It covers the event flow only — UI rendering +(`/ps`, dock, logs overlay) is out of scope. + +There are two layers: + +1. **NotificationService** (`extensions/processes/notifications/service.ts`) + listens to the `ProcessManager` event bus, classifies the end, resolves an + attention level, and emits a language-neutral payload on + `CHANNELS.NOTIFICATION`. +2. **Delivery listener** (`extensions/processes/handlers/notifications.ts`) + subscribes to `CHANNELS.NOTIFICATION`, applies log-match rate limiting, and + converts each payload into a persisted Pi custom message via + `sendProcessNotificationMessage`. + +UI extensions (`processes-logs`, `processes-dock`) also observe +`CHANNELS.NOTIFICATION` for display concerns (e.g. log-match highlighting) but +do not affect delivery. + +## Per-process notify config + +Registered by the `process start` and `process update` tools +(`normalizeNotifyConfig` in `extensions/processes/tools/notify.ts`): + +| Field | Type | Default | +| ------------ | --------------------------------- | --------- | +| `onSuccess` | `"turn"` \| `"context"` \| `"ignore"` | `"turn"` | +| `onFailure` | `"turn"` \| `"context"` \| `"ignore"` | `"turn"` | +| `onKilled` | `"turn"` \| `"context"` \| `"ignore"` | `"context"` | +| `logMatches` | `LogMatcherConfig[]` | `[]` | + +Each `LogMatcherConfig`: + +| Field | Type | Default | +| -------- | --------------------------------- | --------- | +| `pattern` | `string` (≤500 chars, non-empty) | required | +| `mode` | `"literal"` \| `"regex"` | `"literal"` | +| `stream` | `"stdout"` \| `"stderr"` \| `"both"` | `"both"` | +| `repeat` | `boolean` | `false` | +| `on` | `"turn"` \| `"context"` \| `"ignore"` | `"turn"` | + +There are no global settings that affect notifications. The extension config +(`ProcessConfig` in `extensions/processes/config/types.ts`) only covers +execution, output retention, and UI — none of it changes attention or delivery. + +## Attention levels + +An attention level is resolved per event and mapped to Pi send options by +`attentionToSendOptions` (`extensions/processes/notification-sender.ts`): + +| Attention | `triggerTurn` | `deliverAs` | Agent effect | +| --------- | ------------- | ----------- | -------------------------------------------------- | +| `turn` | `true` | `steer` | Wakes an idle agent; the message steers the turn. | +| `context` | `false` | `steer` | Delivered as a steer but does not wake the agent; seen when context returns. | +| `ignore` | `false` | `steer` | Not emitted for lifecycle events (filtered upstream). For log matches, delivered like `context`. | + +Every delivered message has `display: true`, so it is visible in the +conversation regardless of attention. The only difference between `turn` and +`context` is whether the agent is woken. + +## Lifecycle notifications + +`NotificationService.handleProcessEnded` runs on every `process_ended` event +(deferred one microtask so the start tool can register the notify config +first). It classifies the end, resolves attention from the per-process config +or defaults, and either emits or suppresses. + +### Classification + +`classifyProcessEnd` (`extensions/processes/notifications/classify.ts`): + +| `ProcessInfo.status` / `endReason` | Kind | +| ------------------------------------------- | --------- | +| `status === "killed"` | `killed` | +| `success === true` | `success` | +| `endReason` ∈ `spawn_error`/`missing_pid`/`lost` | `crash` | +| `exitCode !== null && exitCode !== 0` | `crash` | +| otherwise | `failure` | + +`terminate_timeout` never reaches `process_ended`: `ProcessManager.kill` sets +`endTime` and transitions to `terminate_timeout` without emitting, and the +`close` handler bails on the `endTime` guard. The timeout is carried by the +tool result (`KillResult.reason: "timeout"`) instead. + +### Attention resolution + +`resolveAttention` picks the config field for the kind, falling back to +`DEFAULT_ATTENTION`: + +| Kind | Config field | Default | +| --------- | ------------ | --------- | +| `success` | `onSuccess` | `turn` | +| `failure` | `onFailure` | `turn` | +| `crash` | `onFailure` | `turn` | +| `killed` | `onKilled` | `context` | + +### Forced display for crashes/failures + +After resolving attention, the service forces a minimum of `context` for +`crash` and `failure`: + +- If attention is `ignore` and the kind is `crash` or `failure`, attention is + upgraded to `context` and the notification is emitted. +- If attention is `ignore` and the kind is `success` or `killed`, the + notification is suppressed entirely (no emit). + +This means `onFailure: "ignore"` cannot fully silence a failed process — it +becomes a context message. `onSuccess: "ignore"` and `onKilled: "ignore"` do +fully silence. + +### Intentional stops (config bypass) + +A stop is *intentional* when it goes through `killIntentionally` +(`extensions/processes/handlers/kill-process.ts`), which calls +`registry.markIntentionalStop(id)` before `manager.kill`. Three entry points +use it: + +- The `process stop` tool. +- The `/ps:kill` command. +- The `/ps` overview panel `x` key (via `requestKill` → + `CHANNELS.COMMAND_KILL`). + +When `handleProcessEnded` sees an intentional stop, it **bypasses the +per-process config entirely** and emits a single `context` notification with +the classified kind. The agent is not woken, but it learns the process ended. +User `onKilled`/`onSuccess`/`onFailure` settings are ignored on this path. + +If the kill fails (`not_found`/`error`) or the process is already dead, the +intentional-stop marker is consumed back and the normal path runs. + +### End-state callstacks + +Each block shows the call chain and the notification it produces. "Emit" means +a `CHANNELS.NOTIFICATION` payload with the shown attention; the delivery +listener then sends it as a Pi message. + +#### Normal success + +``` +child process exits 0 + ProcessRuntimeController.child.on("close") src/manager/process-runtime-controller.ts + transition(managed, "exited") + emit process_ended + NotificationService.handleEvent extensions/processes/notifications/service.ts + queueMicrotask → handleProcessEnded + classifyProcessEnd → "success" + resolveAttention → config.onSuccess ?? "turn" + emit kind=success attention= + delivery listener → sendProcessNotificationMessage(triggerTurn=) +``` + +Default: a turn. `onSuccess: "context"` downgrades to a context message. +`onSuccess: "ignore"` suppresses. + +#### Normal failure (non-zero exit) + +``` +child process exits non-zero + ProcessRuntimeController child.on("close") + transition(managed, "exited") + emit process_ended + handleProcessEnded + classifyProcessEnd → "crash" (exitCode !== 0) + resolveAttention → config.onFailure ?? "turn" + forced display: onFailure==="ignore" → upgraded to "context" + emit kind=crash attention= + delivery listener → sendProcessNotificationMessage +``` + +Default: a turn. `onFailure: "ignore"` becomes a context message (never fully +silent). + +#### Spawn error / missing pid / lost + +``` +spawn error (or liveness tick detects lost/missing_pid) + handleSpawnError / livenessTick + transition(managed, "exited") + emit process_ended + handleProcessEnded + classifyProcessEnd → "crash" + resolveAttention → config.onFailure ?? "turn" + forced display: ignore → "context" + emit kind=crash attention= +``` + +Same as normal failure: defaults to turn, `ignore` forced to context. + +#### External kill (signal from outside the manager) + +``` +external signal kills the process group + child.on("close", code, signal) + transition(managed, "killed") signal branch + emit process_ended + handleProcessEnded + isIntentionalStop? no (no marker) + classifyProcessEnd → "killed" + resolveAttention → config.onKilled ?? "context" + (no forced display for killed) + attention==="ignore" → suppressed; else emit kind=killed + delivery listener → sendProcessNotificationMessage(triggerTurn=) +``` + +Default: a context message (no turn). `onKilled: "turn` promotes to a turn. +`onKilled: "ignore"` suppresses. + +#### Intentional stop (config bypass) + +``` +/ps panel "x" | /ps:kill | process stop tool + requestKill / executeStop + COMMAND_KILL handler extensions/processes/handlers/commands.ts + killIntentionally extensions/processes/handlers/kill-process.ts + registry.markIntentionalStop(id) + manager.kill(id, opts) + transition(managed, "killed"|"exited") + emit process_ended + handleProcessEnded + isIntentionalStop? yes + BYPASS config — emit kind= attention="context" + delivery listener → sendProcessNotificationMessage(triggerTurn=false) +``` + +Always a context message. Config is ignored. The agent is not woken. + +#### Kill timeout (terminate_timeout) + +``` +manager.kill SIGTERM grace period expires, process still alive + ProcessRuntimeController.kill src/manager/process-runtime-controller.ts + endReason = "kill_timeout"; endTime = Date.now() + transition(managed, "terminate_timeout") (no process_ended emit — not exited/killed) + returns KillResult { ok: false, reason: "timeout" } + child.on("close") later → bails on endTime guard, no second process_ended +``` + +No notification. The timeout is reported through the tool/command result only. + +## Log-watch notifications + +`NotificationService.handleOutputChanged` runs on every +`process_output_changed` event. It syncs the compiled matcher set for the +process, evaluates matchers against the appended text, and emits one +`log_match` payload per match with the matcher's `on` attention. + +``` +process emits output + ProcessOutput append + emit process_output_changed { id, appendedText } + NotificationService.handleOutputChanged + registry.getWatchState(id) → null? return + syncMatcherState(id, watchState) + evaluateLogMatchers(matchers, appendedText, now) + per matcher: skip if !repeat && fired; cooldown if repeat+fired + per line: stream filter, plainTextForDisplay, lineMatcher + for each match: + emit kind=log_match attention=match.on + delivery listener → rate-limit check → sendProcessNotificationMessage +``` + +Matcher behavior: + +- `repeat: false` (default) fires once per process lifetime. +- `repeat: true` fires again after a 15s cooldown + (`LOG_MATCH_COOLDOWN_MS`). +- Lines are normalized with `plainTextForDisplay` before matching, so CR + progress lines and escape bytes do not fire watches. +- Lines longer than 10 000 chars are skipped. + +Unlike lifecycle events, log-match `on: "ignore"` is **not** filtered by the +service: it emits on the channel and the delivery listener delivers it like +`context` (displayed steer, no turn). `on: "context"` and `on: "ignore"` are +effectively equivalent for log matches today. + +### Log-match rate limiting + +The delivery listener caps live `log_match` deliveries at 20 per 60s window +(`MAX_LOG_MATCH_NOTIFICATIONS_PER_WINDOW`, `NOTIFICATION_WINDOW_MS`). Overflows +are counted and flushed as a single summary at window end: + +``` +log_match payload arrives + delivery listener + window expired or new? flush prior suppressed summary (kind=log_match_suppressed, attention=context) + sentInWindow >= 20? suppressed++; return + sentInWindow++; sendProcessNotificationMessage + (window timer) flushSuppressedSummary + emit kind=log_match_suppressed summary "Suppressed N log-match notifications..." +``` + +The summary is always `context`. + +## Summary matrix + +| End state | Kind | Default attention | Config field | `ignore` effect | Bypassed by intentional stop? | +| ------------------------ | -------- | ----------------- | ------------ | ---------------------- | ----------------------------- | +| exit 0 | success | turn | `onSuccess` | suppressed | yes → context | +| exit non-zero | crash | turn | `onFailure` | forced to context | yes → context | +| spawn_error/missing/lost | crash | turn | `onFailure` | forced to context | yes → context | +| external signal | killed | context | `onKilled` | suppressed | yes → context | +| intentional stop | (classified) | context | — | — | (this is the bypass) | +| kill timeout | — | — | — | — | no emit (tool result only) | +| log match | log_match | turn | matcher `on` | delivered as context | n/a | +| log-match overflow | log_match_suppressed | context | — | — | n/a | + +## References + +- `extensions/processes/notifications/service.ts` — event handling, attention + resolution, intentional-stop bypass, log-match emission. +- `extensions/processes/notifications/classify.ts` — end classification. +- `extensions/processes/notifications/registry.ts` — per-process config and + intentional-stop markers. +- `extensions/processes/handlers/kill-process.ts` — intentional stop marking. +- `extensions/processes/handlers/notifications.ts` — delivery listener and + rate limiting. +- `extensions/processes/notification-sender.ts` — attention → Pi send options. +- `extensions/processes/tools/notify.ts` — config normalization and defaults. +- `src/manager/process-runtime-controller.ts` — `process_ended` emission, + `terminate_timeout` non-emit, `close`/`error`/liveness paths. diff --git a/extensions/processes/notifications/service.test.ts b/extensions/processes/notifications/service.test.ts index 74a3d1f..029b2bc 100644 --- a/extensions/processes/notifications/service.test.ts +++ b/extensions/processes/notifications/service.test.ts @@ -134,7 +134,7 @@ describe("NotificationService", () => { service.dispose(); }); - it("suppresses killed notification for intentional stop", async () => { + it("emits a context notification for an intentionally killed process", async () => { const fakeManager = createFakeManager(); const spy = createNotificationSpy(); const registry = createNotificationRegistry(); @@ -161,7 +161,11 @@ describe("NotificationService", () => { }); await flushQueuedMicrotasks(); - expect(spy.emitted).toHaveLength(0); + expect(spy.emitted).toHaveLength(1); + expect(spy.emitted[0].attention).toBe("context"); + expect(spy.emitted[0].kind).toBe("killed"); + expect(spy.emitted[0].processId).toBe("proc_1"); + expect(registry.get("proc_1")).toBeNull(); service.dispose(); }); @@ -170,7 +174,7 @@ describe("NotificationService", () => { { name: "successful", success: true, exitCode: 0 }, { name: "failed", success: false, exitCode: 1 }, ])( - "suppresses $name exit notification for intentional stop", + "emits a context notification for an intentionally stopped $name process", async ({ success, exitCode }) => { const fakeManager = createFakeManager(); const spy = createNotificationSpy(); @@ -197,7 +201,8 @@ describe("NotificationService", () => { }); await flushQueuedMicrotasks(); - expect(spy.emitted).toHaveLength(0); + expect(spy.emitted).toHaveLength(1); + expect(spy.emitted[0].attention).toBe("context"); expect(registry.get("proc_1")).toBeNull(); service.dispose(); diff --git a/extensions/processes/notifications/service.ts b/extensions/processes/notifications/service.ts index e905b36..8845c8b 100644 --- a/extensions/processes/notifications/service.ts +++ b/extensions/processes/notifications/service.ts @@ -90,6 +90,9 @@ export function createNotificationService(deps: NotificationServiceDeps): { const kind = classifyProcessEnd(info); if (isIntentionalStop) { + const details = buildLifecycleDetails(info, kind, "context"); + events.emit(CHANNELS.NOTIFICATION, details); + cleanupMatcherState(info.id); registry.unregister(info.id); return;