From 5ae5685f359c396e781258ec585d01fd89e121eb Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Fri, 28 Aug 2026 23:43:00 -0400 Subject: [PATCH 1/3] chore: bump version to 0.4.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7b8d696..532f184 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@pi-vault/pi-subagents", - "version": "0.4.0", + "version": "0.4.1", "type": "module", "description": "Pi extension for delegating focused work to specialized subagents - foreground, background, and multi-agent chains.", "author": "Lanh Hoang ", From b50b7126194d7b3a79a97c7a43dedec3f915c1d6 Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Fri, 28 Aug 2026 23:43:05 -0400 Subject: [PATCH 2/3] docs: update changelog metadata --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd6041a..f43e07a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ All notable changes to this project will be documented in this file. -## Unreleased +## 2026-08-29 - [0.4.1] ### Fixed @@ -13,7 +13,7 @@ All notable changes to this project will be documented in this file. - Align pi-subagents-owned overlays, widgets, fleet navigation, conversation viewing, and inline status rendering with a compact dashboard hierarchy that follows Pi's active theme. - Raise the tested Pi development baseline to `0.84.3`; Pi `0.84.3+` and Node.js `24.15.0+` are required. -## [0.4.0] - 2026-07-17 +## 2026-07-17 - [0.4.0] ### Added @@ -61,7 +61,7 @@ All notable changes to this project will be documented in this file. - Node.js requirement remains `>=24.15.0`. - `timeout_ms` frontmatter (deprecated since 0.3.0) is now fully ignored; migrate to `max_turns`. -## [0.3.0] - 2026-07-05 - 2026-06-14 +## 2026-07-05 - [0.3.0] ### Changed @@ -81,7 +81,7 @@ All notable changes to this project will be documented in this file. - Deduplicated serialized `skills` values when writing agent definitions. - Addressed follow-up code review issues in the extracted subagent execution modules. -## [0.2.0] - 2026-06-14 +## 2026-06-14 - [0.2.0] ### Added @@ -104,7 +104,7 @@ All notable changes to this project will be documented in this file. - Fixed foreground agent rendering so slash agent cards stay live during quiet periods. - Fixed menu selection handling to guard against an undefined selection result. -## [0.1.0] - 2026-06-07 +## 2026-06-07 - [0.1.0] ### Added From 97f0c7b6b3e51c2b73982aa282f3b971f385e94d Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Fri, 28 Aug 2026 23:44:08 -0400 Subject: [PATCH 3/3] chore: remove obsolete plan and specification documents --- ...view-runtime-phase-1-thinking-contracts.md | 167 ----- ...review-runtime-phase-2-model-resolution.md | 197 ----- ...n-review-runtime-phase-3-chain-behavior.md | 165 ---- ...view-runtime-phase-4-session-forwarding.md | 385 ---------- ...-review-runtime-phase-5-chain-preflight.md | 178 ----- ...plan-review-runtime-phase-6-saved-chain.md | 161 ---- ...lan-review-runtime-phase-7-verification.md | 106 --- ...026-08-19-implement-plan-review-runtime.md | 476 ------------ ...ashboard-ui-phase-1-dependency-baseline.md | 278 ------- ...dashboard-ui-phase-2-chain-input-repair.md | 362 --------- ...shboard-ui-phase-3-dashboard-foundation.md | 466 ------------ ...chain-dashboard-ui-phase-4-live-widgets.md | 497 ------------ ...-chain-dashboard-ui-phase-5-agents-menu.md | 446 ----------- ...n-dashboard-ui-phase-6-inline-renderers.md | 706 ------------------ ...n-chain-dashboard-ui-phase-7-fleet-list.md | 441 ----------- ...ashboard-ui-phase-8-conversation-viewer.md | 418 ----------- ...shboard-ui-phase-9-release-verification.md | 364 --------- .../2026-08-26-run-chain-dashboard-ui.md | 670 ----------------- ...8-18-implement-plan-review-chain-design.md | 145 ---- 19 files changed, 6628 deletions(-) delete mode 100644 docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-1-thinking-contracts.md delete mode 100644 docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-2-model-resolution.md delete mode 100644 docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-3-chain-behavior.md delete mode 100644 docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-4-session-forwarding.md delete mode 100644 docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-5-chain-preflight.md delete mode 100644 docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-6-saved-chain.md delete mode 100644 docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-7-verification.md delete mode 100644 docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-1-dependency-baseline.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-2-chain-input-repair.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-3-dashboard-foundation.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-4-live-widgets.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-5-agents-menu.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-6-inline-renderers.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-7-fleet-list.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-8-conversation-viewer.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-9-release-verification.md delete mode 100644 docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui.md delete mode 100644 docs/superpowers/specs/2026-08-18-implement-plan-review-chain-design.md diff --git a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-1-thinking-contracts.md b/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-1-thinking-contracts.md deleted file mode 100644 index a3f2fd1..0000000 --- a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-1-thinking-contracts.md +++ /dev/null @@ -1,167 +0,0 @@ -# Implement Plan Review Runtime — Phase 1: Thinking Contracts Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make saved and inline chain definitions accept, normalize, validate, and serialize per-task thinking levels. - -**Architecture:** Add one small shared lexical normalizer at `src/shared/thinking.ts`, then use the existing chain normalization boundary for saved, executable, append, and inline-chain inputs. This phase defines the chain data contract only; it does not resolve model capabilities, change chain behavior resolution, or forward values into sessions. - -**Tech Stack:** TypeScript, Vitest, TypeBox, Biome, pnpm. - -**Spec:** `docs/superpowers/specs/2026-08-18-implement-plan-review-chain-design.md` - -**Parent plan:** `docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md` - -## Current Repository Fit - -The repository already has `thinking` on agent definitions, single-agent tool input, `RunOptions`, `SpawnOptions`, and the agent runner. Phase 1 must leave those existing contracts and single-agent behavior unchanged. - -The current chain path is different: chain step types, saved-chain field recognition, Markdown/JSON serialization, inline token configuration, and the chain TypeBox schema do not support `thinking`. `resolveStepBehavior()` and `StepSpawnOptions` are intentionally owned by Phase 3, which already plans model/thinking precedence and all three execution branches. Keeping that boundary prevents Phase 1 and Phase 3 from editing the same behavior plumbing. - -## Reference Alignment - -- Pi’s `packages/ai/src/types.ts` defines the same seven-value model-thinking vocabulary (with `off` represented by `ModelThinkingLevel`), while `packages/ai/src/models.ts` owns concrete-model capability discovery. Phase 1 mirrors the vocabulary but does not call the capability helper. -- `tintinweb-pi-subagents/src/types.ts`, `src/agent-manager.ts`, and `src/agent-runner.ts` keep thinking as a typed spawn setting with explicit override precedence over agent configuration. That confirms the existing local single-agent contract should remain untouched here. -- `nicobailon-pi-subagents/src/shared/settings.ts`, `src/runs/shared/parallel-utils.ts`, and `src/runs/foreground/subagent-executor.ts` separate authored chain steps from the resolved runner launch contract. Phase 1 therefore stops at normalized authored data; Phase 3/4 own effective behavior and session forwarding. - -## Contract - -- Accept exactly `off`, `minimal`, `low`, `medium`, `high`, `xhigh`, and `max`, case-insensitively. -- Trim surrounding whitespace before comparison and serialize the normalized lowercase value. -- Preserve omission: an absent `thinking` field remains absent; explicit `thinking: off` remains explicit. -- Reject empty strings, non-strings, and unsupported strings. Errors must identify the source/step and list the accepted levels. -- Per-task placement is supported on `SequentialStep`, `ParallelTaskItem`, and the dynamic parallel template (the `DynamicParallelTemplate` alias inherits the parallel-task field). There is no group-level thinking field. -- Inline syntax is per agent token, for example `scout[thinking=MAX] "scan"`; group suffixes remain group-only options. -- Phase 1 performs lexical validation only. Pi model capability checks and any clamp/reject decision against a concrete model belong to Phase 2. - -The shared helper should expose: - -```ts -export const THINKING_LEVELS = [ - "off", - "minimal", - "low", - "medium", - "high", - "xhigh", - "max", -] as const; - -export type ChainThinkingLevel = (typeof THINKING_LEVELS)[number]; - -export function normalizeThinkingLevel( - value: unknown, - source?: string, -): ChainThinkingLevel; -``` - -`value` is `unknown` because JSON and tool inputs cross a trust boundary. The helper is the single lexical source of truth; it must not inspect a model or silently downgrade a requested level. - ---- - -### Task 1: Add the Shared Thinking Contract - -**Files:** - -- Create: `src/shared/thinking.ts` -- Modify: `src/shared/types.ts` -- Test: `tests/chain-serializer.test.ts` - -- [ ] **Step 1: Add failing contract tests.** - - Extend the existing chain-normalization tests with valid `MAX`/`High` values and invalid empty, `extreme`, `null`, and numeric values. Cover sequential, static parallel, and dynamic-template tasks. Assert normalized lowercase values, preserved omission, explicit `off`, step/source context, and the accepted-level list in failures. - -- [ ] **Step 2: Run the focused serializer tests and confirm failure.** - - ```bash - pnpm vitest run tests/chain-serializer.test.ts - ``` - - Expected: new assertions fail because chain definitions currently reject `thinking` as an unknown field or retain no normalized value. - -- [ ] **Step 3: Implement the helper and type fields.** - - Add the allow-list and normalizer in `src/shared/thinking.ts`. Add `thinking?: ChainThinkingLevel` to `SequentialStep`, `ParallelTaskItem`, and `ChainStepConfig`; the dynamic template alias then carries it without a second field. Do not change the pre-existing agent/single-agent `thinking?: string` fields. - ---- - -### Task 2: Normalize and Serialize Saved Chain Definitions - -**Files:** - -- Modify: `src/core/chain-serializer.ts` -- Test: `tests/chain-serializer.test.ts` - -- [ ] **Step 1: Make the saved parser authoritative for `thinking`.** - - Add `thinking` to the sequential/static/dynamic recognized-field sets. In `validateTaskFields`, reject non-strings and call `normalizeThinkingLevel` for every task shape, including the dynamic template; wrap failures as the existing `ChainDefinitionError` with the source and step label. In `.chain.md`, run the helper even for an empty value so `thinking:` cannot silently become omission. Keep the existing step labels (`step N`, parallel-task labels, dynamic-template labels) in errors. - -- [ ] **Step 2: Update both serializers.** - - Emit `thinking: ` in Markdown when present. Normalize/validate the steps before `serializeJsonChain` writes `chain`, just as `serializeChain` already normalizes its Markdown input; this keeps manually constructed configs subject to the same contract. Preserve ordinary extension fields and all existing output/schema behavior. - -- [ ] **Step 3: Add saved-format tests.** - - Cover `.chain.md` and `.chain.json` parsing, round-trip serialization, uppercase normalization, explicit `off`, static parallel items, and dynamic templates. Assert invalid JSON values fail before materialization and that a serialized config never emits an uppercase level. - -- [ ] **Step 4: Run the focused serializer tests.** - - ```bash - pnpm vitest run tests/chain-serializer.test.ts - ``` - - Expected: all existing chain validation/round-trip tests and the new thinking cases pass. - ---- - -### Task 3: Add Inline Chain Parsing and Tool Schema Support - -**Files:** - -- Modify: `src/core/slash-chain.ts` -- Modify: `src/core/subagent.ts` -- Test: `tests/slash-chain.test.ts` - -- [ ] **Step 1: Add failing inline tests.** - - Assert `parseSingleTaskToken` and `buildChainSteps` accept `thinking=MAX`/`thinking=High` and produce lowercase values for sequential and parallel-group tasks. Add invalid, empty, and unsupported string values such as `thinking=1`; errors must identify the agent/token and accepted levels. Verify ordinary inline metadata and group options remain unchanged. - -- [ ] **Step 2: Run the focused slash-chain tests and confirm failure.** - - ```bash - pnpm vitest run tests/slash-chain.test.ts - ``` - - Expected: inline `thinking` is currently ignored and invalid values are not rejected. - -- [ ] **Step 3: Implement the inline contract.** - - Add `thinking?: ChainThinkingLevel` to `InlineConfig`, normalize it in `parseInlineConfig`, and let `mapParsedStep` carry it into the existing `ChainStep` object. Wrap single-step parsing in the same notification/error path used for multi-step expressions so an invalid `thinking` value does not escape the `/chain` command handler. - - Add `thinking: Type.Optional(Type.String({ description: "Thinking level: off, minimal, low, medium, high, xhigh, max" }))` to `CHAIN_TASK_FIELDS` in `subagent.ts`. TypeBox only enforces the JSON shape here; `normalizeChainSteps` remains the semantic validator for tool chains and chain appends. Leave the already-supported top-level single-agent `thinking` parameter unchanged. - -- [ ] **Step 4: Run focused parser tests.** - - ```bash - pnpm vitest run tests/chain-serializer.test.ts tests/slash-chain.test.ts - ``` - - Expected: saved and inline parsing, normalization, serialization, and rejection tests pass. - ---- - -## Verification and Handoff - -- [ ] Run `pnpm typecheck`. -- [ ] Run `pnpm lint`. -- [ ] Confirm `git diff --check` is clean. -- [ ] Commit only the Phase 1 files: - - ```bash - git add src/shared/thinking.ts src/shared/types.ts src/core/chain-serializer.ts src/core/slash-chain.ts src/core/subagent.ts tests/chain-serializer.test.ts tests/slash-chain.test.ts - git commit -m "feat: add chain thinking configuration" - ``` - -## Phase Result - -Saved, executable, appended, and inline chain definitions can carry a normalized `ChainThinkingLevel`, while omitted values preserve existing defaults. Phase 2 can reuse `normalizeThinkingLevel`; Phase 3 owns behavior precedence and `StepSpawnOptions` forwarding. No model lookup, capability validation, session creation, or single-agent execution behavior changes in this phase. diff --git a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-2-model-resolution.md b/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-2-model-resolution.md deleted file mode 100644 index dd59ca3..0000000 --- a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-2-model-resolution.md +++ /dev/null @@ -1,197 +0,0 @@ -# Implement Plan Review Runtime — Phase 2: Model Resolution Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Resolve explicit model requests to authenticated Pi model objects and validate explicit thinking levels without fallback or clamping. - -**Architecture:** Keep the legacy metadata-only `resolveModel()` unchanged for its existing RPC and scheduling callers. Add a registry-facing resolver that selects one canonical model from Pi’s active registry, checks that it is available, and returns the actual object from `find()`. Keep thinking capability validation as a separate helper so phase 4 and phase 5 can validate an already-selected model without coupling selection to execution precedence. - -**Tech Stack:** TypeScript, `@earendil-works/pi-ai`, `@earendil-works/pi-coding-agent`, Vitest, pnpm. - -**Spec:** `docs/superpowers/specs/2026-08-18-implement-plan-review-chain-design.md` - -**Parent plan:** `docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md` - -## Current Repository Fit - -- `src/core/model-resolver.ts` currently returns `{ id, provider }` metadata and uses first-match fuzzy behavior. Existing callers must keep that function and behavior. -- `src/shared/thinking.ts` already exports `normalizeThinkingLevel()` and `ChainThinkingLevel`; reuse it instead of adding another lexical vocabulary or normalizer. -- `src/core/model-scope.ts` already accepts canonical string IDs. Scope integration belongs to phase 5, after effective chain behavior and preflight traversal exist. -- Pi’s `ExtensionContext.modelRegistry` is the active synchronous registry. Its `getAll()` and `getAvailable()` return model metadata/objects, and `find(provider, id)` returns the runtime model object. - -## Reference Alignment - -- Pi’s `ModelRegistry` exposes synchronous `getAll()`, `getAvailable()`, and `find()`. Pi AI exports `getSupportedThinkingLevels(model)` and defines the runtime object as `Model`. -- `nicobailon-pi-subagents` separates canonical model metadata from runtime selection, normalizes model segments, and never switches providers for a qualified request. -- `tintinweb-pi-subagents` resolves against available models and passes the actual `Model` object into the runner rather than a `{ provider, id }` copy. - -## Global Constraints - -- The registry resolver receives a required `ModelRegistryLike`; missing-registry handling is owned by the later runtime entrypoint tasks. -- Match candidates from `getAll()` before applying availability from `getAvailable()`. A model that exists but is not available must report unavailable; a query with multiple matches must report ambiguous even if only one match is available. -- Matching is case-insensitive and trims surrounding whitespace. Exact provider/ID wins, then exact ID, then a unique fuzzy ID/name match. -- A qualified `provider/id` query never falls back to another provider. If the first segment is not a registered provider, treat the full value as an ID/name so owner/name-style IDs remain valid. -- Return the exact object from `registry.find(provider, id)`. Never reconstruct a model object from metadata. -- Explicit unsupported thinking levels throw. Never call Pi’s clamping helper and never silently fall back to the parent model. -- Preserve the existing `resolveModel(query, models): ResolvedModel | undefined` signature and behavior. - ---- - -### Task 1: Add Strict Registry Model Selection - -**Files:** - -- Modify: `src/core/model-resolver.ts` -- Test: `tests/model-resolver.test.ts` - -**Interfaces:** - -```ts -import type { Api, Model } from "@earendil-works/pi-ai"; - -export interface ModelRegistryLike { - getAll(): ModelInfo[]; - getAvailable(): ModelInfo[]; - find(provider: string, id: string): Model | undefined; -} - -export interface ResolvedModelSelection { - requested: string; - canonical: string; - model: Model; -} - -export function resolveModelSelection( - requested: string, - registry: ModelRegistryLike, -): ResolvedModelSelection; -``` - -- [ ] **Step 1: Add failing registry fixtures and selection tests.** - - Keep the existing `mockModels` tests unchanged. Add a registry fixture whose `getAll()` contains: - - `openai-codex/gpt-5.6-luna`, returned by `find()` as `sentinelModel`; - - `openai/gpt-5.6-luna`, also matching the fuzzy term `luna`; - - one model present in `getAll()` but absent from `getAvailable()`. - - Add assertions for these named cases: - - ```ts - expect(resolveModelSelection("openai-codex/gpt-5.6-luna", registry)).toEqual({ - requested: "openai-codex/gpt-5.6-luna", - canonical: "openai-codex/gpt-5.6-luna", - model: sentinelModel, - }); - expect(resolveModelSelection("gpt-5.6-luna", uniqueIdRegistry).model).toBe( - sentinelModel, - ); - expect(resolveModelSelection("Claude Sonnet", namedRegistry).canonical).toBe( - "anthropic/claude-sonnet-4-20250514", - ); - expect(() => resolveModelSelection("luna", registry)).toThrow(/ambiguous/i); - expect(() => resolveModelSelection("unavailable", registry)).toThrow( - /unavailable/i, - ); - expect(() => resolveModelSelection("missing", registry)).toThrow(/unknown/i); - expect(() => resolveModelSelection(" ", registry)).toThrow(/non-empty/i); - ``` - - Cast only the minimal test sentinel fixture to `Model`; the production return type must remain `Model`. - -- [ ] **Step 2: Run the focused resolver tests and confirm the new assertions fail.** - - ```bash - pnpm vitest run tests/model-resolver.test.ts - ``` - - Expected: the existing metadata tests pass and the new registry tests fail because `resolveModelSelection()` does not exist. - -- [ ] **Step 3: Implement the smallest registry resolver.** - - In `src/core/model-resolver.ts`: - 1. Trim and case-fold the request; reject blank input. - 2. Build canonical candidate keys from `registry.getAll()`, deduplicated by lowercased `provider/id`. - 3. Select one candidate in this order: exact qualified provider/ID, exact ID, then unique fuzzy ID/name. For fuzzy matching, normalize case and cosmetic `.`, `_`, and repeated `-` separators; compare the normalized query against the model ID and optional display name. Restrict qualified queries to their named provider. - 4. Throw an ambiguity error listing canonical candidates when more than one canonical candidate matches. - 5. Compare the selected canonical key against `registry.getAvailable()`. Throw an unavailable error if it is absent. - 6. Call `registry.find(selected.provider, selected.id)`. Throw if the active registry cannot return the selected runtime object. - 7. Return `{ requested, canonical: `${provider}/${id}`, model }`, preserving the trimmed request and the registry’s canonical provider/ID spelling. - - Do not modify `resolveModel()` or `src/core/model-scope.ts`. - -- [ ] **Step 4: Run the resolver tests and verify the selection contract.** - - ```bash - pnpm vitest run tests/model-resolver.test.ts - ``` - - Expected: all metadata and registry-selection tests pass, including sentinel object identity, exact-match precedence, unique fuzzy matching, ambiguity, unknown, unavailable, and blank-request errors. - -- [ ] **Step 5: Commit registry selection.** - - ```bash - git add src/core/model-resolver.ts tests/model-resolver.test.ts - git commit -m "feat: resolve configured models through Pi registry" - ``` - -### Task 2: Add Strict Thinking Capability Validation - -**Files:** - -- Modify: `src/core/model-resolver.ts` -- Test: `tests/model-resolver.test.ts` - -**Interfaces:** - -```ts -import type { Api, Model } from "@earendil-works/pi-ai"; -import type { ChainThinkingLevel } from "../shared/thinking.js"; - -export function validateModelThinking( - model: Model, - canonical: string, - requested: unknown, -): ChainThinkingLevel | undefined; -``` - -- [ ] **Step 1: Add failing capability-validation tests.** - - Use one reasoning model whose `thinkingLevelMap` exposes `max`, one reasoning model without `max`, and one non-reasoning model. Add assertions for: - - `undefined` returns `undefined` and preserves omission; - - `" HIGH "` returns `"high"`; - - explicit `"off"` succeeds for the non-reasoning model; - - `"max"` succeeds only for the model exposing `max`; - - unsupported `"max"`/`"xhigh"` throws an error containing the requested level, canonical model, and supported levels; - - an invalid lexical value throws the existing accepted-level error from `normalizeThinkingLevel()`. - -- [ ] **Step 2: Run the capability tests and confirm failure.** - - ```bash - pnpm vitest run tests/model-resolver.test.ts - ``` - - Expected: the new validation assertions fail because `validateModelThinking()` does not exist. - -- [ ] **Step 3: Implement validation using the existing helpers.** - - Return `undefined` for an omitted request. Otherwise call `normalizeThinkingLevel(requested, "model " + canonical)`, call Pi’s `getSupportedThinkingLevels(model)`, and require the normalized level to be included. Throw an error that names the requested level, canonical model, and comma-separated supported levels when the capability check fails. Do not clamp or replace the request. - -- [ ] **Step 4: Run resolver, scope, and type checks.** - - ```bash - pnpm vitest run tests/model-resolver.test.ts tests/model-scope.test.ts - pnpm typecheck - ``` - - Expected: all focused tests pass and TypeScript accepts the actual `Model` registry object passed to Pi’s capability helper. `tests/model-scope.test.ts` is a regression check only; it is not modified by this phase. - -- [ ] **Step 5: Commit strict thinking validation.** - - ```bash - git add src/core/model-resolver.ts tests/model-resolver.test.ts - git commit -m "feat: validate model thinking capabilities" - ``` - -## Phase Result - -Phase 2 exposes two composable runtime primitives: explicit model strings resolve deterministically to authenticated Pi model objects, and explicit thinking levels normalize and fail strictly against the selected model’s real capabilities. Phase 4 owns forwarding; phase 5 owns chain-wide preflight and canonical scope enforcement. diff --git a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-3-chain-behavior.md b/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-3-chain-behavior.md deleted file mode 100644 index 5b6b57c..0000000 --- a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-3-chain-behavior.md +++ /dev/null @@ -1,165 +0,0 @@ -# Implement Plan Review Runtime — Phase 3: Chain Behavior Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make every chain execution shape calculate and forward its effective raw model and thinking settings. - -**Architecture:** Extend the existing chain behavior resolver and the three execution branches—sequential, static parallel, and dynamic parallel. Phase 3 resolves only step overrides against agent-definition defaults and emits raw strings through `StepSpawnOptions`; Phase 4 owns parent-model fallback, registry lookup, model objects, and session forwarding. - -**Tech Stack:** TypeScript, Vitest, pnpm. - -**Spec:** `docs/superpowers/specs/2026-08-18-implement-plan-review-chain-design.md` - -**Parent plan:** `docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md` - -## Current Repository Fit - -- Phase 1 already adds `thinking?: ChainThinkingLevel` to sequential steps, static parallel items, and dynamic templates, and normalizes authored values before execution. -- Phase 2 already provides strict registry model selection and model-capability thinking validation; neither belongs in this phase. -- `AgentDefinition` already has `model?: string` and `thinking?: string`. -- `src/core/chain-execution.ts` currently resolves only `skills` defaults and forwards only step model strings. Its three builders are the complete raw behavior boundary for this phase; they also provide model provenance to the existing chain scope check. -- `SpawnOptions` already accepts `model?: unknown` and `thinking?: string`; Phase 4 will map the raw chain options to that runtime contract. - -## Reference Alignment - -- `tintinweb-pi-subagents/src/agent-runner.ts` resolves explicit option → agent configuration → parent model and passes the selected model object plus `thinkingLevel` to the session. This phase stops before that runtime resolution. -- `nicobailon-pi-subagents/src/runs/shared/parallel-utils.ts` models each runner step with raw `model?: string` and `thinking?: string`; its foreground executor resolves effective launch values separately. This phase follows the same authored-versus-runtime split. -- `pi/packages/coding-agent/src/core/model-registry.ts` exposes synchronous `getAll()`, `getAvailable()`, and `find()`, while `pi/packages/ai/src/models.ts` owns thinking capability discovery. Those APIs remain Phase 2/4 concerns. - -## Global Constraints - -- For this phase, effective raw precedence is step override, then agent definition. If both are omitted, leave the option undefined so Phase 4 can apply the parent-model/session fallback. -- When an effective model is emitted, set `modelSource` to `"explicit"` only when the step supplies `model`; otherwise set it to `"inherited"` for the agent-definition model. The existing `subagent.ts` chain wrapper consumes this provenance only for model-scope warning/error classification. -- `StepSpawnOptions.model` remains a requested raw model string, and `StepSpawnOptions.thinking` remains a raw/normalized thinking string. Do not resolve registry objects, validate model capabilities, clamp thinking, or create sessions here. -- Preserve existing output, progress, tools, budgets, isolation, concurrency, cancellation, and chain-status behavior. -- Do not modify chain schema/types, serializer/parser code, `/chain` dispatch, `AgentManager`, `agent-runner`, or model resolution in this phase. The only `subagent.ts` change allowed is passing the raw `modelSource` into its existing chain model-scope check. -- Set `model` and `thinking` when their effective values are defined; do not use truthiness checks that accidentally change the raw contract. - ---- - -### Task 1: Propagate Raw Chain Behavior - -**Files:** - -- Modify: `src/core/chain-settings.ts` -- Modify: `src/core/chain-execution.ts` -- Modify: `src/core/subagent.ts` (existing chain model-scope source selection only) -- Test: `tests/chain-execution.test.ts` -- Test: `tests/model-scope.test.ts` - -**Interfaces:** - -```ts -export interface StepSpawnOptions { - toolBudget?: ResolvedToolBudget; - isolation?: "worktree"; - skills?: string[]; - model?: string; - modelSource?: "explicit" | "inherited"; - thinking?: string; - parentSignal?: AbortSignal; -} - -export interface StepOverrides { - output?: string | false; - outputMode?: OutputMode; - reads?: string[] | false; - progress?: boolean; - skills?: string[] | false; - model?: string; - thinking?: string; -} - -export interface ResolvedStepBehavior { - output: string | false; - outputMode: OutputMode; - reads: string[] | false; - progress: boolean; - skills: string[] | false; - model?: string; - thinking?: string; -} - -export interface AgentBehaviorDefaults { - output?: string | false; - reads?: string[] | false; - progress?: boolean; - skills?: string[] | false; - model?: string; - thinking?: string; -} -``` - -- [ ] **Step 1: Extend the test agent fixture and add failing precedence tests.** - - Let the `makeAgentDef` test helper accept `Partial` overrides. Use an agent with: - - ```ts - { model: "agent/model", thinking: "medium" } - ``` - - Capture the fourth `spawnAndWait` argument and cover each execution shape: - - Sequential: run one step with `model: "step/model"` and `thinking: "high"`, followed by an omitted step. Assert the captured options are respectively `{ model: "step/model", modelSource: "explicit", thinking: "high" }` and `{ model: "agent/model", modelSource: "inherited", thinking: "medium" }`. - - Static parallel: run two items for the same agent, one with both overrides and one omitted. Use `concurrency: 1` so call order is deterministic, then assert the same override/default pair and provenance values. - - Dynamic parallel: seed a named structured output with `{"items":["one"]}` and expand `/items`. Run the dynamic template once with both overrides and once without them; assert the first emits explicit provenance and step values and the second emits inherited provenance and agent defaults. - - Keep existing model-only and unrelated execution tests unchanged. The tests must assert that an omitted step receives agent defaults and that no model/thinking fields are invented when both sources omit them. - -- [ ] **Step 2: Run the focused chain tests and confirm the new assertions fail.** - - ```bash - pnpm vitest run tests/chain-execution.test.ts - ``` - - Expected: existing tests pass, while the new thinking/default assertions fail because `StepSpawnOptions` has no thinking field and `agentDefaults()` currently returns only skills. - -- [ ] **Step 3: Add model and thinking to the behavior contract.** - - In `src/core/chain-settings.ts` and `src/core/chain-execution.ts`: - 1. Add `thinking?: string` to `StepOverrides`, `ResolvedStepBehavior`, and `AgentBehaviorDefaults`. - 2. Keep `model?: string` on all three interfaces and add `model: agentDef.model` plus `thinking: agentDef.thinking` to `agentDefaults()` in `src/core/chain-execution.ts`. - 3. Resolve both fields with nullish precedence: - - ```ts - model: overrides.model ?? agentDefaults.model, - thinking: overrides.thinking ?? agentDefaults.thinking, - ``` - - Preserve explicit authored values, including `"off"`, and preserve omission as `undefined`. - -- [ ] **Step 4: Update all three spawn-option builders.** - - Add `thinking?: string` to `StepSpawnOptions`. Pass `thinking: .thinking` into `resolveStepBehavior()` for sequential steps, static parallel items, and dynamic templates. In each branch, set both effective fields without mutating the agent definition: - - ```ts - if (behavior.model !== undefined) { - options.model = behavior.model; - options.modelSource = step.model !== undefined ? "explicit" : "inherited"; - } - if (behavior.thinking !== undefined) options.thinking = behavior.thinking; - ``` - - Use the corresponding raw step field for each builder (`item.model`, `step.parallel.model`, and `seqStep.model`). In `src/core/subagent.ts`, use `options.modelSource` when calling the existing `checkModelScope`; keep its previous fallback classification for non-chain callers. - - Retain the current skills, budget, isolation, parent-signal, prompt, result, and status handling exactly as-is. - -- [ ] **Step 5: Run focused tests and type verification.** - - ```bash - pnpm vitest run tests/chain-execution.test.ts tests/model-scope.test.ts - pnpm typecheck - git diff --check - ``` - - Expected: all chain-execution tests pass, TypeScript accepts the new raw option, and the diff has no whitespace errors. - -- [ ] **Step 6: Commit the phase.** - - ```bash - git add src/core/chain-settings.ts src/core/chain-execution.ts src/core/subagent.ts tests/chain-execution.test.ts tests/model-scope.test.ts - git commit -m "feat: propagate raw chain model and thinking settings" - ``` - -## Phase Result - -Every sequential, static-parallel, and dynamic-parallel execution branch emits the effective raw step/agent model and thinking values, plus explicit/inherited model provenance for scope enforcement. Omitted values remain undefined for Phase 4 to resolve against the parent runtime; no registry, capability, or session behavior changes in this phase. diff --git a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-4-session-forwarding.md b/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-4-session-forwarding.md deleted file mode 100644 index 8fd7212..0000000 --- a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-4-session-forwarding.md +++ /dev/null @@ -1,385 +0,0 @@ -# Implement Plan Review Runtime — Phase 4: Session Forwarding Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Ensure single-agent and chain dispatch paths pass actual Pi model objects and strictly normalized thinking values into agent sessions. - -**Architecture:** Phase 3 already propagates raw effective model and thinking strings through `StepSpawnOptions`; this phase resolves them only at the chain and single-agent runtime boundaries. Reuse phase 2's `resolveModelSelection()` and `validateModelThinking()` primitives, pass the selected `Model` object through `SpawnOptions.model`, and keep `runAgent()` able to resolve configured definition strings for callers that bypass the main subagent tool path. - -The runner uses the precedence established by the reference implementations: an explicit runtime model object wins, then a configured agent-definition model is resolved through Pi's registry, then the parent context model is inherited. The phase does not add a resolver abstraction, migrate legacy RPC lookup, pass Pi's private `modelRuntime`, or move chain validation ahead of execution; those choices keep this change aligned with phase 2 and phase 5. - -**Tech Stack:** TypeScript, Pi Coding Agent SDK, Pi AI, Vitest, pnpm. - -**Spec:** `docs/superpowers/specs/2026-08-18-implement-plan-review-chain-design.md` - -**Parent plan:** `docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md` - -**Reference implementations:** - -- `tintinweb-pi-subagents/src/agent-runner.ts` — explicit runtime model, configured model, then parent-model precedence. -- `nicobailon-pi-subagents/src/runs/shared/model-fallback.ts` — explicit versus inherited provenance and strict failure for unresolved explicit models. -- `packages/coding-agent/src/core/model-registry.ts` and `packages/coding-agent/src/core/sdk.ts` — `getAll()`, `getAvailable()`, `find()`, actual `Model` objects, and `thinkingLevel` session input. - -## Global Constraints - -- `createAgentSession({ model })` receives a real `Model` object or `undefined`; never pass a configured model string to the session. -- Use the existing `ModelRegistryLike` contract: `getAll()`, `getAvailable()`, and `find(provider, id)`. Do not probe for the nonexistent `listModels()` method. -- A configured raw model must resolve through `resolveModelSelection()`. If the registry is unavailable, the model is unknown, ambiguous, unavailable, or has no runtime object, return the existing error path; never replace it with `ctx.model`. -- If no raw model is configured, preserve the parent `ctx.model` fallback. If both are absent and no thinking value is requested, omit `SpawnOptions.model` and preserve Pi's existing default-model behavior. -- If thinking is configured, validate it with `validateModelThinking()` against the selected model. If no selected model exists, fail rather than allow Pi to clamp against an implicit default. Omitted thinking remains omitted. -- Preserve `resolveInvocationConfig()` precedence: agent definition/frontmatter model and thinking take priority over tool parameters, followed by defaults. -- Preserve the raw `AgentDefinition.model` on temporary effective definitions for custom-tool creation and diagnostics. Session selection must come from the resolved `SpawnOptions.model` object. -- Keep `modelSource` and the existing raw-string `checkModelScope()` behavior at the `subagent` chain boundary. Canonical scope preflight remains phase 5. -- Do not modify the phase-3 raw propagation in `src/core/chain-execution.ts` or `src/core/chain-settings.ts`. -- Do not migrate `src/core/rpc.ts` or `src/index.ts`; phase 2 intentionally retains legacy metadata-only RPC lookup. Runner fallback covers RPC/child callers that provide an agent-definition model string. -- Do not add dependencies, a new shared runtime resolver, or a cast to Pi's private `modelRuntime` property. - -### Runtime selection matrix - -| Boundary | Configured value | Selected runtime model | Thinking source | -| ----------------- | ---------------------------------------------------------- | ------------------------------------------------------------- | ----------------------------------------- | -| Chain wrapper | `options.model ?? agentDef.model` | Registry result when configured; otherwise parent `ctx.model` | Effective `options.thinking` from phase 3 | -| Single-agent tool | `resolved.model` from `resolveInvocationConfig()` | Registry result when configured; otherwise parent `ctx.model` | `resolved.thinking` | -| `runAgent()` | `options.model` object, then `agentDef.model`, then parent | Explicit object; registry result; then `ctx.model` | `options.thinking ?? agentDef.thinking` | - ---- - -### Task 1: Forward Resolved Models from Chain Wrappers - -**Files:** - -- Modify: `src/core/slash-chain.ts` — resolve the effective raw step model before `/chain` and saved-chain dispatch calls `spawnAndWait`. -- Modify: `src/core/subagent.ts` — resolve the effective raw step model in the inline `chain` tool path while retaining model-scope provenance. -- Test: `tests/slash-chain.test.ts` — verify the slash-chain wrapper forwards a runtime model object and normalized thinking. -- Test: `tests/subagent-chain.test.ts` — verify the subagent chain wrapper forwards the same runtime model object and normalized thinking. - -**Interfaces:** - -- Consumes `StepSpawnOptions.model?: string`, `StepSpawnOptions.modelSource?: "explicit" | "inherited"`, `StepSpawnOptions.thinking?: string`, and the Pi context's `modelRegistry`/`model` values. -- Produces `SpawnOptions.model?: unknown` containing the `Model` returned by `resolveModelSelection()` and `SpawnOptions.thinking?: string` containing the result of `validateModelThinking()`. - -- [ ] **Step 1: Add failing slash-chain forwarding assertions.** - - Extend the existing `executeSlashChain` test fixture with a registry whose metadata contains `test/model` and whose `find()` returns a reasoning-capable sentinel object. Supply a separate parent model on the command context so accidental parent fallback is observable. Execute a step with `model: "test/model"` and `thinking: "HIGH"`, spy on `manager.spawnAndWait`, and assert identity and normalization: - - ```ts - expect(spawn.mock.calls[0]?.[2]).toMatchObject({ - model: sentinelModel, - thinking: "high", - }); - expect(spawn.mock.calls[0]?.[2]?.model).toBe(sentinelModel); - ``` - - Keep the existing custom-tool assertion and continue asserting that the effective definition passed to `createAgentCustomToolsFactory()` contains the raw `model: "test/model"` string. - -- [ ] **Step 2: Add failing subagent-chain forwarding assertions.** - - Invoke the registered subagent tool with a context containing the same registry and a different parent sentinel. Use a sequential chain step with `model: "test/model"` and `thinking: "HIGH"`, spy on `manager.spawnAndWait`, and assert `options.model` is the registry sentinel by identity and `options.thinking` is `"high"`. Add one failure assertion with an unknown configured model and a parent sentinel; verify the tool returns an error and `spawnAndWait` is not called. - -- [ ] **Step 3: Run the wrapper tests and confirm they fail.** - - ```bash - pnpm vitest run tests/slash-chain.test.ts tests/subagent-chain.test.ts - ``` - - Expected: the current wrappers omit `model` and `thinking`; the new identity assertions fail before implementation. - -- [ ] **Step 4: Resolve and pass the model object in both wrappers.** - - Import `resolveModelSelection` and `validateModelThinking`. At each wrapper, use the phase-3 raw contract: - - ```ts - const rawModel = options?.model ?? agentDef.model; - if (rawModel !== undefined && !ctx.modelRegistry) { - throw new Error( - `Cannot resolve model "${rawModel}": model registry unavailable`, - ); - } - const selection = rawModel - ? resolveModelSelection(rawModel, ctx.modelRegistry) - : undefined; - const selectedModel = selection?.model ?? ctx.model; - const canonical = - selection?.canonical ?? - (selectedModel - ? `${selectedModel.provider}/${selectedModel.id}` - : undefined); - const thinking = - options?.thinking === undefined - ? undefined - : selectedModel && canonical - ? validateModelThinking(selectedModel, canonical, options.thinking) - : (() => { - throw new Error("Cannot validate thinking without an active model"); - })(); - ``` - - Pass `selection?.model` as `SpawnOptions.model` only when a raw model was configured; otherwise leave the field absent so `runAgent()` owns parent fallback. Pass the validated thinking value when defined. Preserve the current temporary `effectiveAgentDef` mutation for raw model metadata and custom-tool creation. In `src/core/subagent.ts`, run the existing raw `checkModelScope()` check before registry resolution and keep its `modelSource` classification unchanged. - -- [ ] **Step 5: Run the wrapper tests and verify the forwarding contract.** - - ```bash - pnpm vitest run tests/slash-chain.test.ts tests/subagent-chain.test.ts - ``` - - Expected: both wrappers pass the same registry sentinel object and `"high"`; unknown configured models fail before spawning; custom-tool definitions still contain raw model strings. - -### Task 2: Resolve and Forward Single-Agent Invocation Settings - -**Files:** - -- Modify: `src/core/subagent.ts` — replace the dead `listModels()` validation with strict registry selection and add model/thinking to single-agent spawn options. -- Test: `tests/subagent.test.ts` — verify manager forwarding, parent fallback preservation, and explicit-model failure. - -**Interfaces:** - -- Consumes `ResolvedInvocationConfig.model?: string` and `.thinking?: string`, the Pi `ExtensionContext.modelRegistry`, and the existing model-scope source calculation. -- Produces `SpawnOptions.model?: unknown` containing the selected runtime model and `SpawnOptions.thinking?: string` containing the validated normalized level. - -- [ ] **Step 1: Add failing single-agent forwarding tests.** - - Add an inline registry fixture with a reasoning-capable sentinel model and a different parent model. Execute the single-agent tool with an agent definition model and uppercase thinking, then assert the manager receives the registry sentinel by identity and `thinking: "high"`: - - ```ts - expect(spawnAndWait.mock.calls[0]?.[2]).toMatchObject({ - model: sentinelModel, - thinking: "high", - }); - expect(spawnAndWait.mock.calls[0]?.[2]?.model).toBe(sentinelModel); - ``` - - Add a no-model/no-thinking case with a parent model and assert the tool does not invent `SpawnOptions.model`; the runner must receive the context for parent fallback. Add an unknown-model case with a parent sentinel and assert the result is an error and no spawn occurs, proving an explicit model cannot silently fall back to the parent. - -- [ ] **Step 2: Run the single-agent tests and confirm they fail.** - - ```bash - pnpm vitest run tests/subagent.test.ts - ``` - - Expected: the current path either skips registry validation because it looks for `listModels()` or omits model/thinking from the spawn options. - -- [ ] **Step 3: Replace legacy single-agent model handling.** - - Replace the `resolveModel()`/`listModels()` block with the existing registry contract: - - ```ts - if (resolved.model !== undefined && !ctx.modelRegistry) { - throw new Error( - `Cannot resolve model "${resolved.model}": model registry unavailable`, - ); - } - const selection = resolved.model - ? resolveModelSelection(resolved.model, ctx.modelRegistry) - : undefined; - const selectedModel = selection?.model ?? ctx.model; - const canonical = - selection?.canonical ?? - (selectedModel - ? `${selectedModel.provider}/${selectedModel.id}` - : undefined); - const thinking = - resolved.thinking === undefined - ? undefined - : selectedModel && canonical - ? validateModelThinking(selectedModel, canonical, resolved.thinking) - : (() => { - throw new Error("Cannot validate thinking without an active model"); - })(); - ``` - - Require `ctx.modelRegistry` whenever `resolved.model` is defined; let the existing tool error handling report resolver failures. Feed `selection?.canonical` into the existing scope check while preserving the current source calculation (`agentDef.model` is inherited, otherwise `params.model` is explicit). Add `selection?.model` and the normalized thinking value to the `spawnOptions` object, omitting both fields when they are undefined. Do not change `resolveInvocationConfig()` or tool-budget precedence. - -- [ ] **Step 4: Run focused single-agent verification.** - - ```bash - pnpm vitest run tests/subagent.test.ts tests/invocation-config.test.ts - ``` - - Expected: sentinel identity, normalized thinking, omitted-model behavior, explicit-model errors, and existing invocation precedence all pass. - -### Task 3: Resolve Configured Models in the Session Runner - -**Files:** - -- Modify: `src/core/agent-runner.ts` — resolve configured definition strings and normalize thinking immediately before session creation. -- Test: `tests/agent-runner.test.ts` — assert session model identity and fallback/error precedence. -- Test: `tests/agent-manager.test.ts` — assert manager preserves model object identity while forwarding to `runAgent()`. - -**Interfaces:** - -- Consumes `RunOptions.model?: unknown`, `RunOptions.thinking?: string`, `AgentDefinition.model?: string`, `AgentDefinition.thinking?: string`, and the runtime context's actual `model` and `modelRegistry`. -- Produces `createAgentSession({ model?: Model, thinkingLevel?: ChainThinkingLevel })` input with no raw model string. - -- [ ] **Step 1: Add failing runner identity and precedence tests.** - - Extend the existing mocked `createAgentSession` tests with a registry sentinel and a different parent sentinel. Cover these cases: - - ```ts - await runAgent( - agentDef, - { ...options, model: explicitModel, thinking: "HIGH" }, - { - model: parentModel, - modelRegistry, - }, - ); - expect(createAgentSession).toHaveBeenCalledWith( - expect.objectContaining({ model: explicitModel, thinkingLevel: "high" }), - ); - - await runAgent( - { ...agentDef, model: "test/model", thinking: "high" }, - options, - { model: parentModel, modelRegistry }, - ); - expect(createAgentSession).toHaveBeenCalledWith( - expect.objectContaining({ model: sentinelModel, thinkingLevel: "high" }), - ); - ``` - - Also assert that an agent with no configured model uses `parentModel` by identity, while a configured model with no registry rejects and never calls `createAgentSession`. Keep the existing session/tool assertions unchanged. - -- [ ] **Step 2: Add the manager model pass-through assertion.** - - Extend the existing thinking pass-through test in `tests/agent-manager.test.ts` with a sentinel `SpawnOptions.model`. Spy on `runAgent()` and assert the exact same object is present in the forwarded `RunOptions`: - - ```ts - expect(spy).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ model: sentinelModel, thinking: "high" }), - expect.anything(), - ); - ``` - -- [ ] **Step 3: Run runner and manager tests and confirm they fail.** - - ```bash - pnpm vitest run tests/agent-runner.test.ts tests/agent-manager.test.ts - ``` - - Expected: `runAgent()` currently chooses `options.model ?? ctx.model`, ignores `agentDef.model`, and passes raw thinking strings without the required model-aware validation. - -- [ ] **Step 4: Implement the runner fallback and strict session input.** - - Import `Api`/`Model` from `@earendil-works/pi-ai`, plus `resolveModelSelection`, `validateModelThinking`, and the existing `ModelRegistryLike` type. Narrow the incoming context once before selection: - - ```ts - const runtimeCtx = ctx as { - model?: Model; - modelRegistry?: ModelRegistryLike; - sessionManager?: { getBranch?: () => unknown[] }; - }; - ``` - - Resolve the effective model in this order, requiring a registry before either configured string is resolved: - - ```ts - let selectedModel = options.model; - let canonical: string | undefined; - - if (typeof selectedModel === "string") { - if (!runtimeCtx.modelRegistry) { - throw new Error( - `Cannot resolve model "${selectedModel}": model registry unavailable`, - ); - } - const selection = resolveModelSelection( - selectedModel, - runtimeCtx.modelRegistry, - ); - selectedModel = selection.model; - canonical = selection.canonical; - } else if (selectedModel === undefined && agentDef.model !== undefined) { - if (!runtimeCtx.modelRegistry) { - throw new Error( - `Cannot resolve model "${agentDef.model}": model registry unavailable`, - ); - } - const selection = resolveModelSelection( - agentDef.model, - runtimeCtx.modelRegistry, - ); - selectedModel = selection.model; - canonical = selection.canonical; - } else if (selectedModel === undefined) { - selectedModel = runtimeCtx.model; - } - ``` - - Require a registry for either configured string branch; do not use the parent model in those error cases. For an object selected from options or inherited from the parent, derive `canonical` as `${provider}/${id}`. Normalize and validate `options.thinking ?? agentDef.thinking` against the selected model, or throw when thinking is requested without a selected model. Pass only the resolved object and normalized `thinkingLevel` to `createAgentSession`; preserve the existing omitted-field behavior for undefined values. - -- [ ] **Step 5: Run runner and manager verification.** - - ```bash - pnpm vitest run tests/agent-runner.test.ts tests/agent-manager.test.ts - ``` - - Expected: explicit runtime objects beat parent models, configured definition strings resolve through the registry, unconfigured agents inherit the parent object, and raw strings never reach `createAgentSession`. - -### Task 4: Run the Phase-4 Regression Matrix - -**Files:** - -- No new source files. Do not modify `chain-execution.ts`, `chain-settings.ts`, `model-resolver.ts`, `invocation-config.ts`, `rpc.ts`, or `index.ts` as part of this phase. - -**Interfaces:** - -- Verifies the phase-2 resolver, phase-3 raw chain propagation, both chain wrappers, the single-agent tool, the manager pass-through, and the session runner as one runtime contract. - -- [ ] **Step 1: Run the focused forwarding suite.** - - ```bash - pnpm vitest run \ - tests/model-resolver.test.ts \ - tests/chain-execution.test.ts \ - tests/slash-chain.test.ts \ - tests/subagent-chain.test.ts \ - tests/subagent.test.ts \ - tests/invocation-config.test.ts \ - tests/agent-manager.test.ts \ - tests/agent-runner.test.ts - ``` - - Expected: all focused forwarding and existing precedence tests pass. - -- [ ] **Step 2: Check for obsolete single-agent lookup code.** - - ```bash - rg -n "listModels|resolveModel\(" src/core/subagent.ts - ``` - - Expected: no matches. `src/core/rpc.ts` may continue using legacy `resolveModel()` by phase-2 design. - -- [ ] **Step 3: Run the repository checks.** - - ```bash - GIT_CONFIG_GLOBAL=/private/tmp/pi-subagents-empty-gitconfig pnpm check - pnpm typecheck - git diff --check - ``` - - Expected: the full test suite passes. Biome may continue reporting the repository's existing warnings, and the Node engine warning may remain under Node 23; neither is part of this phase. - -- [ ] **Step 4: Review the diff for scope.** - - ```bash - git status --short - git diff -- src/core/slash-chain.ts src/core/subagent.ts src/core/agent-runner.ts \ - tests/slash-chain.test.ts tests/subagent-chain.test.ts tests/subagent.test.ts \ - tests/agent-runner.test.ts tests/agent-manager.test.ts - ``` - - Confirm that the diff contains only runtime forwarding, strict model/thinking selection, and its tests; no phase-3 propagation or phase-5 preflight changes. - -- [ ] **Step 5: Commit the phase.** - - ```bash - git add src/core/slash-chain.ts src/core/subagent.ts src/core/agent-runner.ts \ - tests/slash-chain.test.ts tests/subagent-chain.test.ts tests/subagent.test.ts \ - tests/agent-runner.test.ts tests/agent-manager.test.ts - git commit -m "fix: forward resolved models to agent sessions" - ``` - -## Phase Result - -Single-agent, `/chain`, saved-chain, and inline `subagent` dispatches pass actual Pi model objects and strictly validated thinking levels into sessions. Configured model failures remain errors instead of silently inheriting the parent model, while invocations without overrides preserve parent/default behavior. Chain-wide preflight and canonical model-scope enforcement remain deferred to phase 5. diff --git a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-5-chain-preflight.md b/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-5-chain-preflight.md deleted file mode 100644 index df42848..0000000 --- a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-5-chain-preflight.md +++ /dev/null @@ -1,178 +0,0 @@ -# Implement Plan Review Runtime — Phase 5: Chain Preflight Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Reject invalid effective chain models and thinking levels before stage 1, and before any later appended batch can spawn. - -**Architecture:** Add one shared preflight traversal used by the saved/inline slash-chain path and the `subagent` chain path. It reuses phase 2's strict registry resolver and thinking validator plus phase 4's effective precedence, scans sequential/static-parallel/dynamic-template tasks, and runs after final clarification edits. Pass the same preflight callback into `executeChain` so appended batches are checked immediately before they are added to the executable sequence. - -**Tech Stack:** TypeScript, Vitest, pnpm. - -**Spec:** `docs/superpowers/specs/2026-08-18-implement-plan-review-chain-design.md` - -**Parent plan:** `docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md` - -## Current Repository Fit - -- Phase 2 already provides `resolveModelSelection()` and `validateModelThinking()` in `src/core/model-resolver.ts`; do not add another resolver or capability vocabulary. -- Phase 3 emits raw effective `model`, `modelSource`, and `thinking` values from all three `chain-execution.ts` builders. Phase 4 resolves those values again at each spawn boundary. -- `slash-chain.ts` and `subagent.ts` currently normalize and validate agent names before clarification, then execute foreground/background chains. Shared model preflight must be a separate call after the final clarification normalization. -- `subagent.ts` still performs a raw-string model-scope check inside its chain `spawnAndWait` closure. Phase 5 must remove that chain-only check after canonical preflight is wired, or inherited warnings will duplicate and fuzzy names can be classified against the wrong string. Keep the single-agent scope check unchanged. -- `executeChain()` consumes `chain_append` batches after a completed stage. The entrypoints must pass it a preflight callback so an invalid appended batch fails before its first child spawn; the append queue remains responsible only for structural/output-reference validation. - -## Reference Alignment - -- `nicobailon-pi-subagents/src/runs/shared/model-fallback.ts` resolves explicit → agent-definition → parent values, distinguishes explicit versus inherited scope decisions, and refuses unresolved explicit models instead of silently inheriting the parent. Reuse the local phase-2 strictness; do not copy its fallback behavior for invalid explicit values. -- `nicobailon-pi-subagents/src/runs/shared/parallel-utils.ts` keeps raw `model` and `thinking` on each sequential/parallel runner task, including one shared template for dynamic fan-out. Validate the dynamic template once; do not expand items or consume spawn budget during preflight. -- `tintinweb-pi-subagents/src/agent-runner.ts` passes an actual registry model object into the session and uses explicit → configured → parent precedence. The local phase-4 runner already owns this runtime selection; preflight only validates the same effective choice. -- `pi/packages/coding-agent/src/core/model-registry.ts` exposes synchronous `getAll()`, `getAvailable()`, and `find(provider, id)`. `pi/packages/ai/src/models.ts` exposes `getSupportedThinkingLevels(model)`; never use Pi's clamping helper for an authored request. - -## Global Constraints - -- Initial `/chain`, `/run-chain`, and `subagent.chain` requests must reject unknown, ambiguous, unavailable, registry-missing, unsupported-thinking, and explicit scope-blocked models before stage 1 and before `fireAndForgetChain`. -- Every appended batch must run the same callback before `executeChain()` adds it to the executable sequence. A structurally valid append may be accepted into the queue first; a model/thinking failure must still prevent that batch from spawning. -- Effective raw precedence is `step.model ?? agentDef.model`; if both are omitted, use the actual parent model only for thinking validation and preserve existing parent/default execution behavior. -- A pure parent-model inheritance path does not add a new model-scope warning; this preserves the current chain behavior. An agent-definition model is inherited and warns, while a step model is explicit and throws on violation. -- Errors identify the chain location (`step N`, parallel item, or dynamic template), agent name, and the underlying requested/canonical model where available. -- Dynamic preflight validates the template once. It does not expand runtime items, mutate chain definitions, allocate chain directories, consume spawn budget, or execute acceptance commands. -- Interactive clarification is complete before preflight runs. If clarification edits are invalid, return through the existing normalization error path without invoking model lookup. - ---- - -### Task 1: Implement Shared Chain Preflight - -**Files:** - -- Create: `src/core/chain-preflight.ts` -- Test: `tests/chain-preflight.test.ts` - -**Interfaces:** - -```ts -import type { Api, Model } from "@earendil-works/pi-ai"; -import type { AgentDefinition, ChainStep } from "../shared/types.js"; -import type { ModelRegistryLike } from "./model-resolver.js"; -import type { ModelScopeConfig, ModelScopeViolation } from "./model-scope.js"; - -export interface ChainPreflightOptions { - registry?: ModelRegistryLike; - parentModel?: Model; - modelScope?: ModelScopeConfig; - onScopeWarning?: (warning: ModelScopeViolation) => void; -} - -export function preflightChainModels( - steps: ChainStep[], - findAgent: (name: string) => AgentDefinition, - options: ChainPreflightOptions, -): void; -``` - -- [ ] **Step 1: Add failing no-spawn and precedence tests.** - - Create `tests/chain-preflight.test.ts` with a registry fixture returning distinct runtime-model sentinels. Cover sequential, static parallel, and dynamic-template tasks. Assert that step override beats agent-definition model, agent-definition model beats the parent, and omission preserves the parent/default path. - - Add invalid cases for unknown, ambiguous, unavailable, missing registry, unsupported thinking, and explicit out-of-scope models. Use a `spawned = false` sentinel in the integration-shaped cases, assert the helper throws, and assert it remains false. Add a valid pure-parent case, a requested-thinking-with-parent case, and an inherited out-of-scope case that reaches `onScopeWarning` instead of throwing. - - Assert failures contain the step location and agent name; scope messages use canonical `provider/id`, not the fuzzy/raw request. - -- [ ] **Step 2: Run the new tests and confirm failure.** - - ```bash - pnpm vitest run tests/chain-preflight.test.ts - ``` - - Expected: FAIL because no shared traversal exists. - -- [ ] **Step 3: Implement the smallest traversal.** - - Normalization is owned by the callers; this helper receives normalized `ChainStep[]`. Traverse sequential steps, each static parallel item, and each dynamic parallel template. Resolve each agent with `findAgent()` and use a location label such as `step 2 parallel item 1 (worker)` or `step 3 dynamic template (worker)` for every wrapped error, including unknown-agent failures. - - For each task: - 1. Compute `rawModel = task.model ?? agentDef.model` with `!== undefined` semantics. If it is defined, require `options.registry`, call `resolveModelSelection(rawModel, registry)`, and select its returned runtime model/canonical ID. Never fall back to the parent after an explicit/configured resolution failure. - 2. Otherwise, select `options.parentModel` when present and derive its canonical `provider/id`. If no model is selected and the effective thinking value is omitted, continue without registry access. If thinking is requested without a selected model, throw a contextual error before execution. - 3. Validate `task.thinking ?? agentDef.thinking` with `validateModelThinking()` against the selected model. Preserve explicit `off`; do not clamp or invent a value. - 4. Run `checkModelScope()` only for a configured raw model: source is `explicit` when `task.model` is defined, otherwise `inherited` for `agentDef.model`. Throw contextual explicit violations and send contextual inherited warnings through `onScopeWarning`. - - Wrap resolver, thinking, and scope errors without losing their original detail. The helper must not mutate the steps or agent definitions. - -- [ ] **Step 4: Run the focused helper tests.** - - ```bash - pnpm vitest run tests/chain-preflight.test.ts - pnpm typecheck - ``` - - Expected: all task shapes, precedence, strict failures, canonical scope handling, and parent fallback cases pass. - -### Task 2: Wire Preflight to Initial, Background, and Appended Execution - -**Files:** - -- Modify: `src/core/slash-chain.ts` -- Modify: `src/core/subagent.ts` -- Modify: `src/core/chain-execution.ts` -- Test: `tests/slash-chain.test.ts` -- Test: `tests/subagent-chain.test.ts` -- Test: `tests/chain-execution.test.ts` - -**Interfaces:** - -```ts -export interface ChainExecutionParams { - // existing fields... - preflightChain?: (steps: ChainStep[]) => void; -} -``` - -- [ ] **Step 1: Add failing entrypoint and append regressions.** - - In `subagent-chain.test.ts` and `slash-chain.test.ts`, assert that an invalid initial chain does not call `spawnAndWait` and does not call `fireAndForgetChain` for background execution. Add a clarification case where the edited chain is valid/invalid and verify preflight runs only after the final normalized edit. Keep existing runtime model-forwarding assertions. - - In `chain-execution.test.ts`, pass a preflight callback for an appended batch, make it throw for that batch, and assert the first stage runs but the appended stage never calls `spawnAndWait`. - -- [ ] **Step 2: Run the focused regressions and confirm failure.** - - ```bash - pnpm vitest run tests/slash-chain.test.ts tests/subagent-chain.test.ts tests/chain-execution.test.ts - ``` - - Expected: invalid initial chains currently reach the runtime spawn boundary, and appended steps have no preflight callback. - -- [ ] **Step 3: Wire the shared callback at both entrypoints.** - - Keep the existing structural normalization/agent-name validation before clarification. Build one entrypoint-local callback around `preflightChainModels()` using `ctx.modelRegistry`, `ctx.model`, `settings.modelScope`, and a warning handler that emits the existing `model_scope_warning` message. Invoke it after the final clarification normalization and before either foreground `executeChain()` or background `fireAndForgetChain()`. Catch initial preflight errors through the existing command/tool result path and return without dispatching the chain. - - Pass the same callback as `preflightChain` into every foreground/background `executeChain()` call. In `executeChain()`, call it on the batch returned by `consumeChainAppendRequests()` before pushing that batch into `chainSteps` or resolving its templates. Let the existing `finally`/manager lifecycle report an appended-batch preflight failure. - -- [ ] **Step 4: Remove only the duplicate raw chain scope check.** - - Delete the chain-specific `checkModelScope(stepModel, ...)` block inside `subagent.ts`'s chain `spawnAndWait` closure; canonical scope preflight now owns it. Leave the single-agent scope block and all phase-4 model/thinking resolution intact. Do not change `chain-append.ts`'s structural validation or the model resolver. - -- [ ] **Step 5: Run wrapper, append, and type regressions.** - - ```bash - pnpm vitest run \ - tests/chain-preflight.test.ts \ - tests/chain-execution.test.ts \ - tests/chain-append.test.ts \ - tests/subagent-chain.test.ts \ - tests/slash-chain.test.ts \ - tests/model-scope.test.ts - pnpm typecheck - git diff --check - ``` - - Expected: invalid initial and appended batches fail before their first child spawn; valid foreground, background, clarification, inherited-parent, and model-scope flows remain intact. - -- [ ] **Step 6: Commit the phase.** - - ```bash - git add src/core/chain-preflight.ts src/core/slash-chain.ts src/core/subagent.ts src/core/chain-execution.ts \ - tests/chain-preflight.test.ts tests/slash-chain.test.ts tests/subagent-chain.test.ts tests/chain-execution.test.ts - git commit -m "feat: preflight chain models before execution" - ``` - -## Phase Result - -Initial chain submissions and subsequently appended batches fail safely before any invalid child stage spawns. Every sequential, static-parallel, and dynamic-template task uses the same strict model/thinking selection and canonical scope rules, while pure parent-model inheritance and existing valid execution behavior remain unchanged. diff --git a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-6-saved-chain.md b/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-6-saved-chain.md deleted file mode 100644 index b717ae1..0000000 --- a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-6-saved-chain.md +++ /dev/null @@ -1,161 +0,0 @@ -# Implement Plan Review Runtime — Phase 6: Saved Chain Integration Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Add the expected four-stage `implement-plan-review` saved chain after runtime support is complete. - -**Architecture:** Keep the existing saved-chain format and worker agent. The chain uses canonical model IDs, explicit thinking levels, and the four approved skills; all stages share the current tree and receive the plan path through `{task}`. - -**Tech Stack:** Chain Markdown format, TypeScript/Vitest discovery tests, pnpm. - -**Spec:** `docs/superpowers/specs/2026-08-18-implement-plan-review-chain-design.md` - -**Parent plan:** `docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md` - -**Verification prerequisite:** Use Node `>=24.15.0`, as required by `package.json`; the -current checkout's Node `23.11.0` runs the focused checks but emits an unsupported-engine -warning. - -## Global Constraints - -- Use `worker` for all four stages. -- Use `openai-codex/gpt-5.6-luna` with `max` for stages 1, 3, and 4. -- Use `minimax/MiniMax-M3` with `high` for stage 2. -- Keep stages sequential, independent, same-tree, and without worktree isolation or automatic commits. -- Stage 1 edits only the plan; review stages edit product code but not the plan. -- Stage 3 reviews `git diff HEAD`, including all current dirty changes. -- Stage 3 delegates only to the bundled read-only `reviewer` agent; `general-purpose` is not a discovered agent in this package. -- Stage 4 uses `ponytail-review` to identify cuts, then applies only justified product-code cleanups; it does not modify the plan, chain, or agent definitions. - ---- - -### Task 1: Add the Saved Chain and Reviewer Delegation - -**Files:** - -- Create: `chains/implement-plan-review.chain.md` -- Modify: `agents/worker.md` -- Test: `tests/chain-serializer.test.ts` -- Test: `tests/chain-discovery.test.ts` -- Test: `tests/agents.test.ts` -- Test: `tests/subagent-chain.test.ts` - -**Interfaces:** - -- The chain is loaded by the existing `/run-chain implement-plan-review -- path/to/plan.md` flow. -- Add `reviewer` to `worker`’s existing `subagent_agents` list so the correctness-review skill can delegate to the bundled read-only reviewer. - -- [ ] **Step 1: Add failing discovery, assignment, and execution-shaped tests.** - - In `tests/chain-discovery.test.ts`, load the repository's packaged `chains/` directory through `discoverChains()` and assert that `implement-plan-review` is discoverable without diagnostics, contains four sequential `worker` steps, and every step task contains `{task}`. In `tests/agents.test.ts`, discover the packaged agents and assert that `worker.subagentAgents` includes `reviewer`. - - In `tests/chain-serializer.test.ts`, retain focused parser/normalizer coverage for the model, thinking, and skills fields used by the saved chain. In `tests/subagent-chain.test.ts`, add an execution-shaped test that feeds the parsed packaged steps through the existing subagent chain path with registry sentinels for both canonical models. Assert four sequential `spawnAndWait` calls, the exact model/thinking values in spawn options, the exact skills in the effective agent definitions, and that each prompt receives the task path through `{task}`. The `max` sentinel must expose `thinkingLevelMap.max`; the `high` sentinel must expose a supported `high` level. - -The assignment assertion is: - - ```ts - expect(steps.map((step) => [step.model, step.thinking, step.skills])).toEqual([ - ["openai-codex/gpt-5.6-luna", "max", ["brainstorming"]], - ["minimax/MiniMax-M3", "high", ["test-driven-development"]], - ["openai-codex/gpt-5.6-luna", "max", ["requesting-code-review"]], - ["openai-codex/gpt-5.6-luna", "max", ["ponytail-review"]], - ]); - ``` - -- [ ] **Step 2: Run the tests and confirm failure.** - - ```bash - pnpm vitest run tests/chain-serializer.test.ts tests/chain-discovery.test.ts tests/agents.test.ts tests/subagent-chain.test.ts - ``` - - Expected: FAIL because the saved chain and worker delegation do not exist. - -- [ ] **Step 3: Create the four-stage chain.** - - Create `chains/implement-plan-review.chain.md` with this exact structure: - - ```markdown - # Implement Plan Review - - ## worker - - model: openai-codex/gpt-5.6-luna - thinking: max - skills: brainstorming - - Read `{task}`. Improve only that plan until it is implementation-ready. - This is an autonomous chain stage: treat the chain task as approval, do not - ask for approval or pause. Modify only `{task}`; do not modify product code, - tests, agent definitions, or chain files. - - ## worker - - model: minimax/MiniMax-M3 - thinking: high - skills: test-driven-development - - Read and follow `{task}`. Implement the requested runtime changes directly - in the current working tree. Modify only the product source/tests required - by the plan; do not modify `{task}`, agent definitions, or chain files. Use - the existing tree; do not create a worktree. Run the checks specified by the - plan and the repository's native checks. Do not commit automatically. - - ## worker - - model: openai-codex/gpt-5.6-luna - thinking: max - skills: requesting-code-review - - Inspect `git diff HEAD`, including all current dirty changes. Use the - bundled read-only `reviewer` agent—the only permitted correctness-review - delegate—with the plan path and dirty diff in its prompt; do not request a - `general-purpose` or additional reviewer. Fix valid correctness findings in - product code, run relevant verification, and do not modify the plan or - commit changes. - - ## worker - - model: openai-codex/gpt-5.6-luna - thinking: max - skills: ponytail-review - - Inspect the final `git diff HEAD`. Use `ponytail-review` as the complexity - audit, then apply only justified cleanup in product code while preserving - required interfaces and behavior. This stage is write-capable even though - the audit skill is report-only by default. Do not modify the plan, chain, or - agent definitions or commit changes. Run relevant verification after any - cleanup. - ``` - - The prompts must preserve these execution rules: - - - Stage 1: read `{task}`, autonomously make only the plan implementation-ready, and do not modify product code, tests, agent definitions, or chain files. - - Stage 2: read and follow `{task}`, implement directly in the current working tree, modify only required product source/tests, and run plan-specified/native checks without changing the plan or chain setup. - - Stage 3: inspect `git diff HEAD`, delegate only to the bundled read-only `reviewer`, fix valid correctness findings in product code, and verify without committing or modifying the plan. - - Stage 4: inspect the final diff, apply only justified complexity cuts in product code, preserve required interfaces, and verify without modifying the plan, chain, or agent definitions. - - Use `model`, `thinking`, and `skills` values exactly as specified in the assignment test. - -- [ ] **Step 4: Allow worker-to-reviewer delegation.** - - Change only `agents/worker.md`’s `subagent_agents` list to add `reviewer`. - -- [ ] **Step 5: Run saved-chain tests.** - - ```bash - pnpm vitest run tests/chain-serializer.test.ts tests/chain-discovery.test.ts tests/agents.test.ts tests/subagent-chain.test.ts - ``` - - Expected: discovery, parsing, assignment, task substitution, execution wiring, - and worker delegation tests pass. - -- [ ] **Step 6: Commit the integration artifact.** - - ```bash - git add chains/implement-plan-review.chain.md agents/worker.md tests/chain-serializer.test.ts tests/chain-discovery.test.ts tests/agents.test.ts tests/subagent-chain.test.ts - git commit -m "feat: add implement plan review chain" - ``` - -## Phase Result - -The expected saved chain is packaged and executable through the completed runtime, with the approved four-stage model, thinking, and skill assignments. diff --git a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-7-verification.md b/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-7-verification.md deleted file mode 100644 index c9aa3fe..0000000 --- a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime-phase-7-verification.md +++ /dev/null @@ -1,106 +0,0 @@ -# Implement Plan Review Runtime — Phase 7: Verification Implementation Plan - -> **For agentic workers:** This is a verification-only phase. Do not modify source, tests, agents, chains, or this plan while executing it. - -**Goal:** Verify the complete runtime change, saved chain, package contents, committed implementation scope, and current working-tree scope after phases 1–6. - -**Architecture:** This phase changes no product files. It runs the repository's existing checks and confirms the runtime contracts through the tests already added by phases 1–6. A product or test failure is fixed in its owning phase and then reverified here. - -**Tech Stack:** Node `>=24.15.0`, pnpm, Biome, TypeScript, Vitest, and pnpm package dry-run tooling. - -**Spec:** `docs/superpowers/specs/2026-08-18-implement-plan-review-chain-design.md` - -**Parent plan:** `docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md` - -## Reference alignment - -- `/Users/lanh/Developer/pi-packages/nicobailon-pi-subagents/src/runs/shared/model-fallback.ts` and `src/runs/shared/parallel-utils.ts` validate effective explicit/configured/parent model selection and retain raw model/thinking settings on runner tasks. The existing local tests cover those contracts; this phase adds no second smoke harness. -- `/Users/lanh/Developer/pi-packages/tintinweb-pi-subagents/src/agent-runner.ts` resolves an actual registry model before session creation, while its package scripts separate lint, typecheck, and full tests. The local `pnpm check` remains the authoritative combined command. -- `/Users/lanh/Developer/pi-packages/pi/packages/coding-agent/src/core/model-registry.ts` exposes synchronous `getAll()`, `getAvailable()`, and `find()` lookups, and the Pi package tests use `getSupportedThinkingLevels()` for capability behavior. The local resolver/preflight tests use those same boundaries. -- The reference packages keep tests out of their published package boundary. This repository's `package.json.files` publishes `src`, `agents`, and `chains`, so the dry-run must verify runtime files and integration artifacts, not `tests/`. - -## Global constraints - -- Require Node `>=24.15.0`; a lower version is a precondition failure, not a passing verification result. -- Do not run live model calls or `acceptance.command`; all runtime coverage is local and uses existing test doubles/sentinels. -- Do not add tests, scripts, dependencies, acceptance gates, retries, commits, or generated files. -- Do not run formatting or auto-fix commands. Existing unrelated Biome warnings do not authorize source cleanup in this phase; only a non-zero check result blocks verification. -- Preserve the parent plan and all phase artifacts. Review any current dirty changes with `git diff HEAD`. -- The implementation base recorded before Phase 1 is `9619442b0208ac16299f1423db2ab959410c1e87`. Review committed Phase 1–6 scope against that fixed base; do not derive the range from `HEAD~N`. - ---- - -### Task 1: Run complete verification - -**Files:** - -- Modify: none -- Test: all files under `tests/` through `pnpm check` - -**Interfaces:** - -- No new interfaces. This phase verifies the contracts delivered by phases 1–6. - -- [ ] **Step 1: Check the toolchain before running the suite.** - - ```bash - node --version - pnpm --version - ``` - - Expected: Node is `v24.15.0` or newer. Stop and report the environment if it is older; do not sign off on the phase using an unsupported engine. - -- [ ] **Step 2: Run the complete check command with signing disabled for child test repositories.** - - ```bash - GIT_CONFIG_COUNT=1 \ - GIT_CONFIG_KEY_0=commit.gpgsign \ - GIT_CONFIG_VALUE_0=false \ - pnpm check - ``` - - The environment override is process-local and prevents host-level Git signing settings from breaking tests that create temporary repositories. It does not change repository configuration. - - Expected: Biome lint, TypeScript compilation, and the complete Vitest suite exit with status 0. Pre-existing Biome warnings may be reported because the repository's `lint` script does not treat warnings as errors. - -- [ ] **Step 3: Run the package dry run and verify the published boundary.** - - ```bash - pnpm pack:dry-run - ``` - - Expected: the package list includes `chains/implement-plan-review.chain.md`, `agents/worker.md`, and all runtime files under `src/`, including `src/core/chain-preflight.ts` and `src/shared/thinking.ts`. It must not be used to expect files under `tests/`; `package.json.files` intentionally excludes them. - -- [ ] **Step 4: Verify whitespace, committed scope, dirty scope, and plan immutability.** - - First, review the committed Phase 1–6 range using the recorded implementation base: - - ```bash - git diff --check 9619442b0208ac16299f1423db2ab959410c1e87..HEAD - git diff --stat 9619442b0208ac16299f1423db2ab959410c1e87..HEAD - git diff --name-only 9619442b0208ac16299f1423db2ab959410c1e87..HEAD - ``` - - Confirm that the committed range contains only the expected Phase 1–6 artifacts: the phase plan documents, runtime files under `src/`, tests, and the saved chain/agent artifacts. This historical committed-scope review is separate from the current dirty-scope review below. - - Then review the current working tree and index: - - ```bash - git diff --check - git diff --cached --check - git diff HEAD --stat - git diff HEAD --name-only - git diff --exit-code HEAD -- docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md - git diff --cached --exit-code -- docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md - git status --short - ``` - - Confirm that there are no whitespace errors, no staged or unstaged edits to the parent plan, and no unrelated dirty files. If this review has not been committed, the only allowed pre-existing dirty file is this Phase 7 plan itself. The current dirty-scope review uses `git diff HEAD` because it is the actual handoff scope; the committed-scope review above uses the recorded implementation base rather than a moving `HEAD~N` range. - -- [ ] **Step 5: Close verification without a commit.** - - If a product or test check fails after the toolchain precondition is satisfied, do not patch it here. Return the failure to the phase that owns the behavior, make the smallest fix there, and rerun this phase. Leave the working tree exactly as received after verification. - -## Phase result - -The complete runtime and saved chain pass the supported-toolchain checks, the package dry run contains the intended runtime and chain/agent artifacts, the committed Phase 1–6 range is within its expected scope, the parent plan is unchanged, and no unrelated working-tree changes remain. diff --git a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md b/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md deleted file mode 100644 index c478e96..0000000 --- a/docs/superpowers/plans/2026-08-19-implement-plan-review-runtime.md +++ /dev/null @@ -1,476 +0,0 @@ -# Implement Plan Review Runtime Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Add runtime support for per-step model and thinking propagation so the saved `implement-plan-review` chain runs each stage with its configured Pi model and thinking level. - -**Architecture:** Keep model and thinking values as strings in agent and chain definitions, then resolve explicit models once against Pi's active registry into actual `Model` objects at the runtime boundary. Share the resolver and preflight logic across single-agent, inline-chain, saved-chain, sequential, parallel, and dynamic execution paths; keep the saved chain as the integration artifact produced by those capabilities. - -**Tech Stack:** TypeScript, Pi Coding Agent SDK `@earendil-works/pi-coding-agent`, Pi AI model registry, Vitest, Biome, pnpm. - -**Spec:** `docs/superpowers/specs/2026-08-18-implement-plan-review-chain-design.md` - -## Global Constraints - -- Use the existing `@pi-vault/pi-subagents` package and its bundled agents. -- Use canonical saved-chain models `openai-codex/gpt-5.6-luna` and `minimax/MiniMax-M3`. -- Resolve explicit models before stage 1; unknown, ambiguous, unavailable, or unsupported requests must fail without spawning a stage. -- Pass the actual Pi `Model` object to session creation; never pass a model query string as the session model. -- Effective precedence is step override, then agent definition, then parent model. -- Keep stages sequential, independent, and in the current working tree; do not inherit the parent conversation or create a worktree. -- Review all current dirty changes with `git diff HEAD`; do not create automatic commits, retries, rollback, or recovery behavior. -- Stage 1 may edit only the supplied plan; review stages may edit product code but not the plan. -- `acceptance.command` remains definition metadata and is not executed by the runtime. -- Do not add dependencies, new agent roles, runtime acceptance gates, or parallelism. -- Preserve existing behavior when no model or thinking override is configured. - ---- - -### Task 1: Add Thinking-Level Contracts and Parsing - -**Files:** - -- Modify: `src/shared/types.ts` -- Modify: `src/core/chain-settings.ts` -- Modify: `src/core/chain-serializer.ts` -- Modify: `src/core/slash-chain.ts` -- Modify: `src/core/subagent.ts` -- Test: `tests/chain-serializer.test.ts` -- Test: `tests/slash-chain.test.ts` - -**Interfaces:** - -- Add `thinking?: string` to `SequentialStep`, `ParallelTaskItem`, `DynamicParallelTemplate`, `ChainStepConfig`, and inline chain configuration. -- Add `thinking?: string` to `StepOverrides`, `ResolvedStepBehavior`, `AgentBehaviorDefaults`, and `StepSpawnOptions`. -- Keep these fields string-valued at parsing/configuration boundaries; normalize them to lowercase runtime values. -- Accept exactly `off`, `minimal`, `low`, `medium`, `high`, `xhigh`, and `max`, case-insensitively. - -- [ ] **Step 1: Add failing serializer tests for Markdown and JSON thinking fields.** - - Extend `tests/chain-serializer.test.ts` with a Markdown fixture containing `thinking: MAX` and a JSON fixture containing `thinking: "High"`. Assert both normalize to lowercase and that serialization emits the normalized value. - - Also add invalid cases for `thinking: extreme`, `thinking: ""`, and non-string JSON values; assert the error identifies the step and the accepted values. - -- [ ] **Step 2: Run the focused serializer tests and confirm the new assertions fail.** - - Run: - - ```bash - pnpm vitest run tests/chain-serializer.test.ts - ``` - - Expected: existing tests pass and the new thinking assertions fail because the field is currently rejected or discarded. - -- [ ] **Step 3: Add the thinking field to all parser and behavior types.** - - Update the recognized field sets in `chain-serializer.ts`, parse `thinking:` in `.chain.md`, include it in JSON normalization/serialization, and add `thinking=` to `parseInlineConfig`. Reuse one normalization function so saved and inline chains have identical validation. - -- [ ] **Step 4: Thread thinking through behavior resolution.** - - Update `resolveStepBehavior` so `overrides.thinking ?? agentDefaults.thinking` is returned. Add `thinking` to the inline-to-step mapping and to the TypeBox chain task schema used by the `subagent` tool. - -- [ ] **Step 5: Run the focused tests and confirm they pass.** - - Run: - - ```bash - pnpm vitest run tests/chain-serializer.test.ts tests/slash-chain.test.ts - ``` - - Expected: all focused parser, serializer, normalization, and inline-thinking tests pass. - -- [ ] **Step 6: Commit the parsing contract.** - - ```bash - git add src/shared/types.ts src/core/chain-settings.ts src/core/chain-serializer.ts src/core/slash-chain.ts src/core/subagent.ts tests/chain-serializer.test.ts tests/slash-chain.test.ts - git commit -m "feat: add chain thinking configuration" - ``` - -### Task 2: Implement Pi Model Resolution and Capability Validation - -**Files:** - -- Modify: `src/core/model-resolver.ts` -- Test: `tests/model-resolver.test.ts` - -**Interfaces:** - -- Preserve `resolveModel(query, models): ResolvedModel | undefined` for canonical metadata and existing callers. -- Add a registry-facing resolver with this contract: - - ```ts - export interface ModelRegistryLike { - getAll(): ModelInfo[]; - getAvailable(): ModelInfo[]; - find(provider: string, id: string): unknown | undefined; - } - - export interface ResolvedModelSelection { - requested: string; - canonical: string; - model: unknown; - } - - export function resolveModelSelection( - requested: string, - registry: ModelRegistryLike, - ): ResolvedModelSelection; - ``` - -- The returned `model` must be the actual object returned by `registry.find`, not `{ provider, id }` metadata. -- Add one shared thinking normalizer and one capability check using Pi's supported thinking levels; explicit unsupported levels must throw instead of relying on Pi's native clamping. -- Import `getSupportedThinkingLevels` from `@earendil-works/pi-ai`; do not duplicate Pi's model-capability table. - -- [ ] **Step 1: Add failing resolver tests for exact, fuzzy, ambiguous, unavailable, and actual-object resolution.** - - Extend `tests/model-resolver.test.ts` with a registry fixture whose `getAll()` contains two models matching `"luna"`, whose `getAvailable()` contains only one canonical model, and whose `find()` returns a sentinel object. Assert: - - ```ts - expect(resolveModelSelection("openai-codex/gpt-5.6-luna", registry)).toEqual({ - requested: "openai-codex/gpt-5.6-luna", - canonical: "openai-codex/gpt-5.6-luna", - model: sentinelModel, - }); - expect(() => resolveModelSelection("luna", registry)).toThrow(/ambiguous/i); - expect(() => resolveModelSelection("unavailable", registry)).toThrow( - /available/i, - ); - expect(() => resolveModelSelection("missing", registry)).toThrow(/unknown/i); - ``` - -- [ ] **Step 2: Run the resolver tests and confirm the new cases fail.** - - Run: - - ```bash - pnpm vitest run tests/model-resolver.test.ts - ``` - - Expected: the existing metadata tests pass and the new registry-object assertions fail because the current resolver has no registry object or ambiguity policy. - -- [ ] **Step 3: Implement unique model selection against Pi's registry.** - - Use this precedence: - 1. exact `provider/id` match; - 2. exact model ID match; - 3. unique fuzzy ID/name match. - - Reject zero matches, multiple fuzzy matches, missing registry methods, and models absent from `getAvailable()`. Build the canonical string from the registry result and obtain the runtime object with `find(provider, id)`. - -- [ ] **Step 4: Implement strict thinking normalization and capability checks.** - - Normalize case-insensitively to `off`, `minimal`, `low`, `medium`, `high`, `xhigh`, or `max`. Use Pi's `getSupportedThinkingLevels(model)` for explicit values and throw an error naming the requested level, canonical model, and supported levels when the value is not supported. - -- [ ] **Step 5: Preserve canonical model-scope matching.** - - Keep `checkModelScope` string-based and pass it the canonical `provider/id`, without a thinking suffix. Preserve explicit-model errors and inherited-model warnings. - -- [ ] **Step 6: Run the focused resolver and scope tests.** - - ```bash - pnpm vitest run tests/model-resolver.test.ts tests/model-scope.test.ts - ``` - - Expected: all tests pass, including actual-object identity and strict failure cases. - -- [ ] **Step 7: Commit the resolver.** - - ```bash - git add src/core/model-resolver.ts tests/model-resolver.test.ts - git commit -m "feat: resolve configured models through Pi registry" - ``` - -### Task 3: Propagate Effective Models and Thinking Through Execution - -**Files:** - -- Modify: `src/core/chain-settings.ts` -- Modify: `src/core/chain-execution.ts` -- Modify: `src/core/slash-chain.ts` -- Modify: `src/core/subagent.ts` -- Modify: `src/core/agent-runner.ts` -- Test: `tests/chain-execution.test.ts` -- Test: `tests/subagent-chain.test.ts` -- Test: `tests/subagent.test.ts` -- Test: `tests/agent-manager.test.ts` - -**Interfaces:** - -- `StepSpawnOptions` continues to carry the requested `model?: string`, gains `thinking?: string`, and carries `modelSource?: "explicit" | "inherited"` for the existing chain model-scope classification; it is the chain executor's raw behavior contract. -- `AgentManager.spawn`/`spawnAndWait` continue receiving `SpawnOptions.model?: unknown`; callers must provide the resolved Pi model object at the runtime boundary. -- `runAgent` must resolve any configured string fallback before calling `createAgentSession`, so `createAgentSession({ model })` always receives an actual model object. - -- [ ] **Step 1: Add failing chain behavior tests for model and thinking precedence.** - - Extend `tests/chain-execution.test.ts` with an agent definition containing `model: "agent/model"` and `thinking: "medium"`. Execute sequential, static-parallel, and dynamic steps with and without overrides, and assert the captured options are: - - ```ts - expect(received[0]).toMatchObject({ model: "step/model", thinking: "high" }); - expect(received[1]).toMatchObject({ - model: "agent/model", - thinking: "medium", - }); - ``` - - Add equivalent assertions for each execution path. - -- [ ] **Step 2: Run the chain execution tests and confirm the new assertions fail.** - - ```bash - pnpm vitest run tests/chain-execution.test.ts - ``` - - Expected: current model tests pass, while thinking/default assertions fail because only skills and step model strings are currently propagated. - -- [ ] **Step 3: Include agent model and thinking in chain defaults and all spawn-option builders.** - - Update `agentDefaults` to return `agentDef.model` and `agentDef.thinking`. Pass `thinking` into `resolveStepBehavior`, then set both `model` and `thinking` on sequential, static-parallel, and dynamic `StepSpawnOptions` when defined. - -- [ ] **Step 4: Resolve and forward actual models in both chain dispatch wrappers.** - - In `executeSlashChain` and the `subagent` chain path, resolve the effective raw model with `resolveModelSelection` before calling `deps.manager.spawnAndWait`. Pass the returned `.model` object as `SpawnOptions.model` and the normalized thinking value as `SpawnOptions.thinking`. Do not rely on a temporary `AgentDefinition.model` string for session selection. - -- [ ] **Step 5: Fix the single-agent runtime boundary.** - - In the single-agent path, resolve the effective model from `resolveInvocationConfig` and include the actual model object plus normalized thinking in `spawnOptions`. In `runAgent`, retain a fallback resolver for child/RPC callers that still provide an agent-definition model string, then pass the resolved object to `createAgentSession`. - -- [ ] **Step 6: Add manager/session forwarding assertions.** - - Extend `tests/subagent.test.ts` and `tests/agent-manager.test.ts` with a sentinel model registry and assert that the same sentinel model object reaches `spawnAndWait` and the mocked `createAgentSession`, alongside the requested thinking level. - -- [ ] **Step 7: Run all propagation tests.** - - ```bash - pnpm vitest run tests/chain-execution.test.ts tests/subagent-chain.test.ts tests/subagent.test.ts tests/agent-manager.test.ts - ``` - - Expected: all sequential, parallel, dynamic, chain-wrapper, and single-agent forwarding tests pass. - -- [ ] **Step 8: Commit runtime propagation.** - - ```bash - git add src/core/chain-settings.ts src/core/chain-execution.ts src/core/slash-chain.ts src/core/subagent.ts src/core/agent-runner.ts tests/chain-execution.test.ts tests/subagent-chain.test.ts tests/subagent.test.ts tests/agent-manager.test.ts - git commit -m "fix: propagate chain models and thinking levels" - ``` - -### Task 4: Add Strict Chain Model Preflight - -**Files:** - -- Create: `src/core/chain-preflight.ts` -- Modify: `src/core/slash-chain.ts` -- Modify: `src/core/subagent.ts` -- Modify: `src/core/chain-execution.ts` -- Test: `tests/chain-preflight.test.ts` -- Test: `tests/slash-chain.test.ts` -- Test: `tests/subagent-chain.test.ts` -- Test: `tests/chain-execution.test.ts` - -**Interfaces:** - -- Add: - - ```ts - import type { Api, Model } from "@earendil-works/pi-ai"; - import type { ModelRegistryLike } from "./model-resolver.js"; - - interface ChainPreflightOptions { - registry?: ModelRegistryLike; - parentModel?: Model; - modelScope?: ModelScopeConfig; - onScopeWarning?: (warning: ModelScopeViolation) => void; - } - - function preflightChainModels( - steps: ChainStep[], - findAgent: (name: string) => AgentDefinition, - options: ChainPreflightOptions, - ): void; - ``` - -- `preflightChainModels` scans sequential steps, static parallel items, and dynamic templates. It resolves `step.model ?? agentDef.model`, validates effective thinking against the selected model, and invokes `checkModelScope` with the canonical model. -- It throws before any spawn when a configured model needs a missing registry, or for an unresolved/ambiguous model, unsupported thinking, or explicit scope violation. Chains that only inherit the parent model retain the existing parent-model behavior. -- The entrypoints pass a `preflightChain` callback into `executeChain`; appended batches run it before they are added to the executable sequence. Remove only the duplicate raw chain scope check in `subagent.ts`; keep the single-agent scope path and phase-4 runtime resolution. - -- [ ] **Step 1: Add failing no-spawn preflight tests.** - - Create `tests/chain-preflight.test.ts` with valid precedence/parent cases and invalid cases for unknown, ambiguous, unavailable, missing registry, unsupported-thinking, and scope-blocked models. Use a `spawned = false` sentinel and assert it remains false when preflight throws. Add wrapper no-spawn coverage and an appended-batch callback regression in the listed tests. - -- [ ] **Step 2: Run the new preflight test and confirm it fails.** - - ```bash - pnpm vitest run tests/chain-preflight.test.ts - ``` - - Expected: FAIL because neither chain dispatch path currently performs complete preflight. - -- [ ] **Step 3: Implement the shared preflight scan.** - - Traverse every effective task shape, look up its agent, resolve configured models with Task 2's resolver, validate thinking capability, and report step number, agent name, requested model, and canonical model in errors. Do not spawn or mutate the chain. Validate dynamic templates once and use the actual parent model only when needed for thinking validation. - -- [ ] **Step 4: Invoke preflight after final chain normalization/clarification.** - - Call it in both `/run-chain`/`/chain` execution and `subagent` chain execution immediately before foreground execution or `fireAndForgetChain`. For interactive clarification, run it after the user-edited chain is normalized; for background execution, run it before launching the background callback. Pass the same callback into `executeChain` and invoke it for each consumed append batch before that batch is pushed into execution. - -- [ ] **Step 5: Run preflight and regression tests.** - - ```bash - pnpm vitest run tests/chain-preflight.test.ts tests/chain-execution.test.ts tests/chain-append.test.ts tests/subagent-chain.test.ts tests/slash-chain.test.ts tests/model-scope.test.ts - pnpm typecheck - git diff --check - ``` - - Expected: invalid chains fail before the first `spawnAndWait`, while valid chains preserve existing clarification and background behavior. - -- [ ] **Step 6: Commit preflight.** - - ```bash - git add src/core/chain-preflight.ts src/core/slash-chain.ts src/core/subagent.ts src/core/chain-execution.ts tests/chain-preflight.test.ts tests/slash-chain.test.ts tests/subagent-chain.test.ts tests/chain-execution.test.ts - git commit -m "feat: preflight chain models before execution" - ``` - -### Task 5: Add the Expected Saved Chain and Worker Review Permission - -**Files:** - -- Create: `chains/implement-plan-review.chain.md` -- Modify: `agents/worker.md` -- Test: `tests/chain-serializer.test.ts` -- Test: `tests/subagent-chain.test.ts` - -**Interfaces:** - -- The saved chain is consumed by the existing `/run-chain -- ` loader; it does not introduce a new chain format or execution API. -- Add `reviewer` to the worker's existing `subagent_agents` list so `requesting-code-review` can delegate to the bundled read-only reviewer. - -- [ ] **Step 1: Add failing saved-chain discovery and execution tests.** - - Assert that the packaged chain is discoverable as `implement-plan-review`, contains four sequential `worker` steps, and has these exact model/thinking/skill assignments: - - ```ts - expect(steps.map((step) => [step.model, step.thinking, step.skills])).toEqual( - [ - ["openai-codex/gpt-5.6-luna", "max", ["brainstorming"]], - ["minimax/MiniMax-M3", "high", ["test-driven-development"]], - ["openai-codex/gpt-5.6-luna", "max", ["requesting-code-review"]], - ["openai-codex/gpt-5.6-luna", "max", ["ponytail-review"]], - ], - ); - ``` - -- [ ] **Step 2: Run the saved-chain tests and confirm they fail.** - - ```bash - pnpm vitest run tests/chain-serializer.test.ts tests/subagent-chain.test.ts - ``` - - Expected: FAIL because the new chain file does not yet exist. - -- [ ] **Step 3: Create the saved chain with autonomous stage prompts.** - - Use this exact structure: - - ```markdown - --- - name: implement-plan-review - description: Prepare, implement, and review a plan in the current working tree - --- - - ## worker - - phase: Plan readiness - label: Make plan implementation-ready - model: openai-codex/gpt-5.6-luna - thinking: max - skills: brainstorming - - Read the repository and {task}. The chain invocation is already approved: do not pause for approval or ask the parent a question. Edit only {task} when it is incomplete, contradictory, or not implementation-ready. Do not modify product code. Return only after the plan is implementation-ready. - - ## worker - - phase: Implementation - label: Implement the plan - model: minimax/MiniMax-M3 - thinking: high - skills: test-driven-development - - Read and follow {task}. Implement it directly in the current working tree without creating a worktree or replacing the plan with a new design. Run the plan's tests and relevant repository-native checks. - - ## worker - - phase: Correctness review - label: Review and fix correctness issues - model: openai-codex/gpt-5.6-luna - thinking: max - skills: requesting-code-review - - Read {task}, inspect git diff HEAD including all current dirty changes, and review for defects, regressions, and missing coverage. Use the permitted read-only reviewer delegation when the skill requests it. Apply valid findings directly to product code, never the plan, and verify each fix. Treat HEAD as the review base and the uncommitted working tree as the review head; do not create a commit. - - ## worker - - phase: Simplicity review - label: Remove unjustified complexity - model: openai-codex/gpt-5.6-luna - thinking: max - skills: ponytail-review - - Read {task} and inspect the final git diff. Review only for unnecessary complexity, then apply valid simplifications directly to product code while preserving required interfaces. Do not modify the plan. Verify the final state and do not create a commit. - ``` - -- [ ] **Step 4: Add `reviewer` to `agents/worker.md` and run saved-chain tests.** - - Change only the `subagent_agents` list to include `reviewer`, then run: - - ```bash - pnpm vitest run tests/chain-serializer.test.ts tests/subagent-chain.test.ts - ``` - - Expected: all saved-chain parsing, assignment, and execution tests pass. - -- [ ] **Step 5: Commit the integration artifact.** - - ```bash - git add chains/implement-plan-review.chain.md agents/worker.md tests/chain-serializer.test.ts tests/subagent-chain.test.ts - git commit -m "feat: add implement plan review chain" - ``` - -### Task 6: Run Full Verification and Packaging Checks - -**Files:** - -- Modify: none; corrective edits belong in the originating task before this verification task is rerun -- Test: the complete existing Vitest suite - -**Interfaces:** - -- No new interfaces. This task verifies the contracts produced by Tasks 1–5 and checks that the package contains the saved chain and agent files. - -- [ ] **Step 1: Run the complete check command.** - - ```bash - pnpm check - ``` - - Expected: Biome lint, TypeScript compilation, and all Vitest tests exit with status 0. - -- [ ] **Step 2: Run the package dry run.** - - ```bash - pnpm pack:dry-run - ``` - - Expected: the package file list includes `chains/implement-plan-review.chain.md`, the modified `agents/worker.md`, and all required `src` files; no generated or unrelated files are added. - -- [ ] **Step 3: Inspect the final diff and verify scope.** - - ```bash - git diff --check - git status --short - git diff --stat HEAD~5..HEAD - ``` - - Confirm that only runtime propagation, resolver/preflight logic, tests, the saved chain, and the worker delegation allowlist changed. If any earlier task required a corrective edit, run the narrowest affected test again before declaring verification complete. - -## Implementation Handoff - -After each task commit, review the diff before starting the next task. Use `superpowers:subagent-driven-development` for fresh-agent task execution and review, or `superpowers:executing-plans` for inline execution with checkpoints. Do not invoke either execution skill until the plan is approved for implementation. diff --git a/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-1-dependency-baseline.md b/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-1-dependency-baseline.md deleted file mode 100644 index 6efa2f2..0000000 --- a/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-1-dependency-baseline.md +++ /dev/null @@ -1,278 +0,0 @@ -# Phase 1: Complete Dependency Baseline Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Update every direct development dependency to the approved latest stable version and align the repository metadata, CI runtime, documentation, and lockfile without changing product behavior. - -**Architecture:** Treat the dependency set as one atomic toolchain baseline. Resolve the eight direct development dependencies together, keep the three Pi peer dependencies wildcarded, and update only metadata that must agree with the new versions. The exact dependency set has already passed an isolated Node `24.15.0` compatibility probe, so product source and tests remain unchanged. - -**Tech Stack:** Node.js `24.15.0+`, pnpm `11`, TypeScript `7.0.2`, Vitest `4.1.11`, Biome `2.5.10`, TypeBox `1.3.19`, Pi/TUI `0.84.3`. - -**Spec:** `docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui.md#approved-design` - -## Global Constraints - -- Use the exact local Pi release tag `v0.84.3` in `/Users/lanh/Developer/pi-packages/pi` as the compatibility reference; do not copy from its unreleased `main` branch. -- The tested Pi/TUI baseline is exactly `0.84.3`; retain wildcard Pi peer dependencies and document Pi `0.84.3+` for users. -- Update all eight direct development dependencies to the exact caret ranges listed in Task 1. -- Retain the existing Node engine requirement `>=24.15.0` and make both CI workflows test that runtime. -- Do not change commands, schemas, settings, rendering, interaction behavior, runtime dependencies, or product source/tests. -- Do not add pi-status or any other dependency. -- Keep the parent plan and Phases 2–9 unchanged. -- If the verified dependency set requires a product source change during implementation, stop and report the difference instead of expanding this phase. - ---- - -## Audited Dependency Set - -These were the npm `latest` versions and exact publication dates when this plan was approved on 2026-08-26: - -| Package | Current | Target | Published (UTC) | -| --------------------------------- | --------: | -------: | --------------- | -| `@earendil-works/pi-ai` | `0.80.10` | `0.84.3` | 2026-08-24 | -| `@earendil-works/pi-coding-agent` | `0.80.10` | `0.84.3` | 2026-08-24 | -| `@earendil-works/pi-tui` | `0.80.10` | `0.84.3` | 2026-08-24 | -| `@biomejs/biome` | `2.5.4` | `2.5.10` | 2026-08-21 | -| `@types/node` | `26.1.1` | `26.3.0` | 2026-08-24 | -| `typebox` | `1.3.6` | `1.3.19` | 2026-08-25 | -| `typescript` | `6.0.3` | `7.0.2` | 2026-07-08 | -| `vitest` | `4.1.10` | `4.1.11` | 2026-08-18 | - -Sources: npm package metadata and the local Pi `v0.84.3` changelogs. TypeScript `7.0.2` is intentionally included despite being a compiler major: this repository invokes only `tsc`, does not import the unavailable TypeScript 7 compiler API, and the isolated all-target probe passed typechecking and the complete test suite. - -## File Structure - -- Modify `package.json`: declare the eight approved direct development dependency ranges; leave engines and peer dependencies unchanged. -- Modify `pnpm-lock.yaml`: resolve the declared versions and their transitive graph. -- Modify `biome.json`: match the configuration schema URL to Biome `2.5.10`. -- Modify `pnpm-workspace.yaml`: remove the inert Pi `0.80.10` release-age exclusions and pin patched transitive development dependencies reported by `pnpm audit`. -- Modify `.github/workflows/quality.yml`: run quality checks on Node `24.15.0`. -- Modify `.github/workflows/release.yml`: build and publish on Node `24.15.0`. -- Modify `README.md`: state the tested Pi and Node prerequisites at the install boundary. - -### Task 1: Update and verify the complete dependency baseline - -**Files:** - -- Modify: `package.json:50-63` -- Modify: `pnpm-lock.yaml` -- Modify: `biome.json:2` -- Modify: `pnpm-workspace.yaml:5-9` -- Modify: `.github/workflows/quality.yml:22-27` -- Modify: `.github/workflows/release.yml:23-29` -- Modify: `README.md:10-16` -- Test: all files under `tests/` through `pnpm check` - -**Interfaces:** - -- Consumes: npm-published direct dependencies and the Pi `v0.84.3` release contracts. -- Produces: a lockfile and CI baseline in which Pi packages resolve to `0.84.3`, Biome to `2.5.10`, Node types to `26.3.0`, TypeBox to `1.3.19`, TypeScript to `7.0.2`, and Vitest to `4.1.11`. -- Public APIs/types: none; the three Pi `peerDependencies` remain `"*"`. - -- [ ] **Step 1: Verify the supported toolchain and clean baseline.** - - Run: - - ```bash - node --version - pnpm --version - git status --short - ``` - - Expected: Node is `v24.15.0` or newer, pnpm is major version `11`, and the status contains no unexpected implementation changes. If the active Node is older, use the repository owner's installed runtime for subsequent commands: - - ```bash - mise exec node@24.15.0 -- node --version - ``` - - Expected: `v24.15.0`. - -- [ ] **Step 2: Record the pre-update verification result.** - - Run with Git signing disabled only for child processes that create temporary repositories: - - ```bash - GIT_CONFIG_COUNT=1 \ - GIT_CONFIG_KEY_0=commit.gpgsign \ - GIT_CONFIG_VALUE_0=false \ - mise exec node@24.15.0 -- pnpm check - ``` - - Expected: Biome lint and `tsc --noEmit` complete, then all `56` test files and `1,276` tests pass. Existing Biome warnings are allowed because the current `lint` script does not treat warnings as errors. If this baseline fails for a reason other than environment configuration, record the failure and stop rather than folding an unrelated fix into this phase. - -- [ ] **Step 3: Update all direct development dependency declarations and the lockfile.** - - Run: - - ```bash - mise exec node@24.15.0 -- pnpm add -D \ - @earendil-works/pi-ai@0.84.3 \ - @earendil-works/pi-coding-agent@0.84.3 \ - @earendil-works/pi-tui@0.84.3 \ - @biomejs/biome@2.5.10 \ - @types/node@26.3.0 \ - typebox@1.3.19 \ - typescript@7.0.2 \ - vitest@4.1.11 \ - --lockfile-only - ``` - - Expected: `package.json` contains exactly these ranges: - - ```json - { - "devDependencies": { - "@biomejs/biome": "^2.5.10", - "@earendil-works/pi-ai": "^0.84.3", - "@earendil-works/pi-coding-agent": "^0.84.3", - "@earendil-works/pi-tui": "^0.84.3", - "@types/node": "^26.3.0", - "typebox": "^1.3.19", - "typescript": "^7.0.2", - "vitest": "^4.1.11" - } - } - ``` - - Leave `engines.node` as `">=24.15.0"` and leave all three Pi peer dependencies as `"*"`. - -- [ ] **Step 4: Synchronize the Biome configuration schema.** - - In `biome.json`, change only the schema URL: - - ```json - "$schema": "https://biomejs.dev/schemas/2.5.10/schema.json" - ``` - - Do not run `biome migrate`; the current configuration is valid and only its schema version is stale. - -- [ ] **Step 5: Remove the inert release-age exclusions.** - - Delete this complete block from `pnpm-workspace.yaml`: - - ```yaml - minimumReleaseAgeExclude: - - "@earendil-works/pi-agent-core@0.80.10" - - "@earendil-works/pi-ai@0.80.10" - - "@earendil-works/pi-coding-agent@0.80.10" - - "@earendil-works/pi-tui@0.80.10" - ``` - - Do not replace the entries with `0.84.3`; no `minimumReleaseAge` policy is configured, so the exclusions have no effect. Add the targeted advisory overrides found during implementation review: - - ```yaml - overrides: - "nanoid@<3.3.18": "3.3.18" - "postcss@<=8.5.22": "8.5.26" - ``` - - Both targets satisfy their existing parent ranges: PostCSS `8.5.26` satisfies Vite's `^8.5.16`, and Nano ID `3.3.18` satisfies PostCSS's `^3.3.16`. Do not add either package as a direct dependency. - -- [ ] **Step 6: Align both CI workflows with the declared Node engine.** - - In `.github/workflows/quality.yml` and `.github/workflows/release.yml`, change the existing setup-node input to: - - ```yaml - node-version: "24.15.0" - ``` - - Leave action versions, pnpm `11.3.0`, triggers, caching, commands, permissions, and release steps unchanged. - -- [ ] **Step 7: Document the tested host boundary.** - - In `README.md`, insert this exact sentence immediately after `## Install` and before the install command: - - ```markdown - Requires Pi 0.84.3+ and Node.js 24.15.0+. - ``` - - Do not document dashboard behavior in this phase. - -- [ ] **Step 8: Install exactly from the regenerated lockfile.** - - Run: - - ```bash - mise exec node@24.15.0 -- pnpm install --frozen-lockfile - ``` - - Expected: installation succeeds without changing `package.json` or `pnpm-lock.yaml` further. - -- [ ] **Step 9: Verify versions and eliminate stale declarations.** - - Run: - - ```bash - mise exec node@24.15.0 -- pnpm list --depth 0 - rg -n '0\.80\.10|2\.5\.4|26\.1\.1|1\.3\.6|6\.0\.3|4\.1\.10' \ - package.json pnpm-workspace.yaml biome.json - rg -n '@earendil-works/pi-(agent-core|ai|coding-agent|tui)@0\.80\.10' pnpm-lock.yaml - ``` - - Expected: `pnpm list` reports the eight exact resolved versions from Step 3. Both `rg` commands exit with status `1` and print no matches, proving the superseded direct declarations, stale configuration values, and old Pi lockfile graph are absent. `pnpm-lock.yaml` contains only `@types/node@26.3.0`; the deduplication gate in Step 10 confirms no satisfiable duplicate remains. - -- [ ] **Step 10: Run the complete post-update verification.** - - Run: - - ```bash - GIT_CONFIG_COUNT=1 \ - GIT_CONFIG_KEY_0=commit.gpgsign \ - GIT_CONFIG_VALUE_0=false \ - mise exec node@24.15.0 -- pnpm check - ``` - - Expected: Biome no longer reports a schema-version mismatch, TypeScript `7.0.2` typechecking succeeds, and all `56` test files and `1,276` tests pass. No product source/test edits are permitted to obtain this result. - - Run the dependency advisory gate: - - ```bash - mise exec node@24.15.0 -- pnpm audit --audit-level high - mise exec node@24.15.0 -- pnpm dedupe --check - ``` - - Expected: both commands exit with status `0`, with no High or Critical vulnerabilities and no deduplicatable lockfile entries. - -- [ ] **Step 11: Verify the package boundary.** - - Run: - - ```bash - mise exec node@24.15.0 -- pnpm run pack:dry-run - ``` - - Expected: the dry run succeeds and includes `src/`, bundled agents, bundled chains, `LICENSE`, `CHANGELOG.md`, and `README.md`; it does not include tests or planning documents. - -- [ ] **Step 12: Audit whitespace and exact scope.** - - Run: - - ```bash - git diff --check - git diff --stat - git diff -- \ - package.json \ - pnpm-lock.yaml \ - pnpm-workspace.yaml \ - biome.json \ - README.md \ - .github/workflows/quality.yml \ - .github/workflows/release.yml - git status --short - ``` - - Expected: no whitespace errors; only the seven named dependency-baseline files are modified; commands, schemas, settings, UI code, runtime code, tests, `peerDependencies`, and `engines.node` are unchanged. - -- [ ] **Step 13: Commit the atomic baseline.** - - ```bash - git add \ - package.json \ - pnpm-lock.yaml \ - pnpm-workspace.yaml \ - biome.json \ - README.md \ - .github/workflows/quality.yml \ - .github/workflows/release.yml - git commit -m "chore: update development dependencies" - ``` diff --git a/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-2-chain-input-repair.md b/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-2-chain-input-repair.md deleted file mode 100644 index dbd2b2b..0000000 --- a/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-2-chain-input-repair.md +++ /dev/null @@ -1,362 +0,0 @@ -# Phase 2: Chain Input Repair Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking. - -**Goal:** Make /run-chain and /chain previews respond to modern CSI-u/Kitty keyboard input and prevent a failed preview from launching a chain. - -**Architecture:** Replace raw byte comparisons in ChainClarifyComponent with the Pi/TUI matchesKey() and native Input primitives, forward focus through Focusable, and gate the shared slash-chain preview to TUI mode. Preserve the current preview appearance; dashboard styling starts in Phase 3. - -**Tech Stack:** TypeScript, @earendil-works/pi-ai@0.84.3, @earendil-works/pi-coding-agent@0.84.3, @earendil-works/pi-tui@0.84.3, Vitest, pnpm, Node 24.15.0. - -**Spec:** docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui.md#approved-design - -## Global Constraints - -- Keep the Pi development packages and lockfile at 0.84.3. -- Preserve /run-chain, /chain, prompt-workflow execution, --yes, --bg, schemas, and ChainClarifyResult. -- Use the exact preview condition ctx.mode === "tui" && !bg && !yes in executeSlashChain(). -- A normal cancel (undefined result or { action: "cancel" }) returns silently and does not dispatch. -- A synchronous or rejected preview failure reports one visible pi-subagent-result error and does not dispatch. -- Do not introduce dashboard primitives, frame glyphs, viewport logic, or visual changes; those belong to Phase 3. -- The separate clarify path in src/core/subagent.ts is out of scope because it has a different structured tool-result contract. -- Run automated checks with Node 24.15.0; the current shell is Node 23.11.0 and is below the package engine requirement. - ---- - -## Current repository fit - -- src/tui/chain-clarify.ts currently has a raw switch over legacy bytes and a hand-rolled edit buffer. Its 22 tests cover only legacy input. -- executeSlashChain() in src/core/slash-chain.ts is the shared path for /chain, /run-chain, and prompt-workflow callers. Its preview currently checks only !bg && !yes, and a rejected ctx.ui.custom() promise escapes before dispatch. -- tests/slash-chain.test.ts already owns executor integration and manager spawn assertions. tests/core/chain-clarify-integration.test.ts currently owns only stripExecutionFlags() coverage, so it remains unchanged unless a command-boundary regression is strictly needed. -- The Phase 1 dependency baseline is already merged at HEAD aee51bc; all three Pi development packages and the lockfile resolve to 0.84.3. - -## Reference alignment - -- /Users/lanh/Developer/pi-packages/pi, tag v0.84.3: packages/tui/src/keys.ts supplies matchesKey() and isKeyRelease(); packages/tui/src/components/input.ts supplies setValue(), onSubmit, onEscape, focused, and CSI-u text decoding; the extension-input and llama/ui examples forward wrapper focus to their active Input; ExtensionContext.mode is tui, rpc, json, or print. -- /Users/lanh/Developer/pi-vault/pi-status: src/tui/dashboard.ts demonstrates matchesKey() plus Input/Focusable. src/tui/overlay-render.ts and src/tui/dashboard-layout.ts are presentation references for Phase 3 only. -- /Users/lanh/Developer/pi-packages/nicobailon-pi-subagents: src/tui/fleet.ts and src/slash/slash-commands.ts confirm release-safe key handling and the custom-component boundary; no chain-preview implementation exists there. -- /Users/lanh/Developer/pi-packages/tintinweb-pi-subagents: src/ui/fleet-list.ts uses matchesKey()/isKeyRelease() at an input boundary; its fleet state is unrelated to this phase. - -### Task 1: Reproduce modern input failures - -**Files:** - -- Test: tests/chain-clarify.test.ts - -**Interfaces:** - -- Use the existing makeComponent() helper and ChainClarifyComponent constructor. Do not change production code in this task. - -- [ ] **Step 1: Add the failing CSI-u action tests.** - - Keep the legacy tests and add these exact cases: - - ```ts - test("CSI-u Enter key returns run action", () => { - const { component, result } = makeComponent([ - { agent: "scout", task: "analyze" }, - ]); - component.handleInput("\x1b[13u"); - expect(result.value?.action).toBe("run"); - }); - - test("CSI-u j moves the selection", () => { - const { component } = makeComponent([ - { agent: "scout", task: "analyze" }, - { agent: "planner", task: "plan" }, - ]); - component.handleInput("\x1b[106u"); - expect( - component - .render(80) - .some((line) => line.includes(">") && line.includes("planner")), - ).toBe(true); - }); - - test("CSI-u b returns background action", () => { - const { component, result } = makeComponent([ - { agent: "scout", task: "analyze" }, - ]); - component.handleInput("\x1b[98u"); - expect(result.value?.action).toBe("bg"); - }); - ``` - -- [ ] **Step 2: Add the release-event regression.** - - Send "\x1b[13;1:3u" to a fresh component and assert result.value is still undefined. This must remain true after matchesKey() is introduced, so a Kitty key release cannot confirm the chain. - -- [ ] **Step 3: Run the red test.** - - Run: - - ```bash - mise exec node@24.15.0 -- pnpm vitest run tests/chain-clarify.test.ts - ``` - - Expected: the three CSI-u action tests fail against the raw switch; the release test may already pass because the raw switch ignores that sequence. - -### Task 2: Repair input and focus at the boundary - -**Files:** - -- Modify: src/tui/chain-clarify.ts -- Test: tests/chain-clarify.test.ts - -**Interfaces:** - -- ChainClarifyComponent continues to expose the existing constructor, handleInput(), invalidate(), render(), and ChainClarifyResult variants. -- ChainClarifyComponent additionally implements Focusable through a focused getter/setter and keeps one native Input instance for edit mode. - -- [ ] **Step 1: Replace the raw input boundary with Pi/TUI primitives.** - - Use the installed 0.84.3 API: - - ```ts - import { - Input, - isKeyRelease, - Key, - matchesKey, - type Component, - type Focusable, - type TUI, - } from "@earendil-works/pi-tui"; - - private input = new Input(); - private _focused = false; - - get focused(): boolean { - return this._focused; - } - - set focused(value: boolean) { - this._focused = value; - this.input.focused = value; - } - - dispose(): void { - this.input.focused = false; - this.input.onSubmit = undefined; - this.input.onEscape = undefined; - } - ``` - - Make the class implement Component and Focusable. At the start of handleInput(), return when isKeyRelease(data) is true. In list mode use matchesKey(data, Key.enter), Key.escape, Key.up, and Key.down plus matchesKey(data, "j"), "k", "q", "b", "e", and "m". Preserve the current bounds, actions, result shapes, requestRender() calls, render wording, and > selection marker. - -- [ ] **Step 2: Delegate edit behavior to the native Input.** - - Keep the existing edit-task/edit-model mode state and override maps, but remove editBuffer and its manual mutation from handleInput(). When entering edit mode, seed the native input without sending the seed through key handling: - - ```ts - this.input.setValue( - mode === "edit-task" - ? (this.taskOverrides.get(this.selectedIndex) ?? seq.task ?? "") - : (this.modelOverrides.get(this.selectedIndex) ?? seq.model ?? ""), - ); - this.input.onSubmit = (value) => { - if (this.mode === "edit-task") { - this.taskOverrides.set(this.selectedIndex, value); - } else { - this.modelOverrides.set(this.selectedIndex, value); - } - this.mode = "list"; - this.tui.requestRender(); - }; - this.input.onEscape = () => { - this.mode = "list"; - this.tui.requestRender(); - }; - ``` - - After setValue(), send the native End binding "\x1b[F" through input.handleInput() so a prefilled edit starts with the cursor at the end. Forward all edit-mode data other than the release guard to input.handleInput(data). Render input.render(width) in the existing edit layout so the native cursor marker, cursor movement, deletion, paste, submit, and escape behavior come from Pi/TUI. Do not add frames, the Phase 3 selection glyph, viewport scrolling, or a dashboard footer. - -- [ ] **Step 3: Replace tests that assert the hand-rolled cursor.** - - Retain the existing edit/result/parallel coverage, but assert the edited value rather than the old trailing underscore. Add focused cases for CSI-u printable text (for example "\x1b[104u" and "\x1b[105u"), Kitty arrow aliases "\x1b[1;1B" and "\x1b[1;1A", native backspace "\x7f", CSI-u Enter submit, Escape discard, and focus propagation: - - ```ts - component.handleInput("e"); - component.handleInput("\x1b[104u"); - component.handleInput("\x1b[105u"); - component.handleInput("\x7f"); - component.focused = true; - expect(component.render(80).join("\n")).toContain(CURSOR_MARKER); - ``` - - Also assert focused=false removes the marker, parallel steps remain read-only, legacy keys remain supported, and the release event never resolves the component. - -- [ ] **Step 4: Run the focused green checks.** - - Run: - - ```bash - mise exec node@24.15.0 -- pnpm vitest run tests/chain-clarify.test.ts - mise exec node@24.15.0 -- pnpm typecheck - ``` - - Expected: all legacy and CSI-u component tests pass and the component remains type-safe against Pi/TUI 0.84.3. - -### Task 3: Gate previews and fail closed - -**Files:** - -- Modify: src/core/slash-chain.ts -- Test: tests/slash-chain.test.ts - -**Interfaces:** - -- Keep executeSlashChain() parameters, command registration, dispatch functions, and pi-subagent-result message shape unchanged. -- Test the shared executeSlashChain() boundary once; /chain and /run-chain both delegate to it, so duplicate command fixtures add no coverage. Retain the existing /run-chain saved-chain materialization test. - -- [ ] **Step 1: Add a TUI factory test that exercises CSI-u confirmation.** - - In the existing executeSlashChain validation tests, create a ui.custom spy that captures its options, invokes the supplied factory with a minimal TUI/theme/keybinding fixture, sends "\x1b[13u" to the returned component, and resolves through the supplied done callback. With ctx.mode set to "tui", assert a valid chain calls manager.spawnAndWait exactly once, ui.custom receives overlay: true, and overlayOptions remains { anchor: "center", width: 84, maxHeight: "80%" } with no dashboard-specific options. - - The fixture shape is: - - ```ts - let seenOptions: { overlay?: boolean; overlayOptions?: unknown }; - const custom = vi.fn( - ( - factory: ( - tui: TUI, - theme: Theme, - keybindings: KeybindingsManager, - done: (result: ClarifyResult) => void, - ) => ChainClarifyComponent, - options: { overlay?: boolean; overlayOptions?: unknown }, - ) => { - seenOptions = options; - return new Promise((resolve) => { - const component = factory( - { requestRender: vi.fn() } as TUI, - { - fg: (_name: string, text: string) => text, - bold: (text: string) => text, - } as Theme, - {} as KeybindingsManager, - resolve, - ); - component.handleInput("\x1b[13u"); - }); - }, - ); - ``` - - Add these type-only imports to the test: - - ```ts - import type { KeybindingsManager } from "@earendil-works/pi-coding-agent"; - import type { TUI } from "@earendil-works/pi-tui"; - import type { - ChainClarifyComponent, - ChainClarifyResult, - } from "../src/tui/chain-clarify.js"; - import type { Theme } from "../src/tui/agent-widget.js"; - ``` - - Use the existing createDeps(), AgentManager, completedRecord(), and model registry setup from nearby tests rather than adding a new helper file. - -- [ ] **Step 2: Add no-dispatch cancellation and failure tests.** - - Cover this exact matrix with manager spies: - - custom resolves { action: "cancel", steps }: no spawn, no fire-and-forget, no pi-subagent-result message. - - custom resolves undefined: no spawn, no fire-and-forget, no pi-subagent-result message. - - custom rejects new Error("preview failed"): executeSlashChain() resolves without throwing, no spawn, no fire-and-forget, and exactly one message matches: - - ```ts - expect(pi.sendMessage).toHaveBeenCalledWith( - expect.objectContaining({ - customType: "pi-subagent-result", - content: expect.stringContaining("preview failed"), - display: true, - }), - ); - ``` - - - custom synchronously throws new Error("preview failed"): the same one-message/no-dispatch assertions as the rejected promise case. - -- [ ] **Step 3: Add mode and flag bypass assertions.** - - Add mode: "rpc" to a valid no-flag case and assert the chain executes while ui.custom is not called. Update the existing preview test named preflights only the final clarification edit to use mode: "tui". In TUI mode, assert yes=true bypasses preview and preserves foreground spawn, while bg=true bypasses preview and preserves fire-and-forget dispatch. Keep the existing saved-chain materialization test unchanged. - -- [ ] **Step 4: Implement the single shared gate and error boundary.** - - Replace the current !bg && !yes condition with this exact policy, retaining the existing final normalization and dispatch blocks after the preview: - - ```ts - if (ctx.mode === "tui" && !bg && !yes) { - const { ChainClarifyComponent } = await import("../tui/chain-clarify.js"); - let result: ClarifyResult | undefined; - try { - result = await ctx.ui.custom( - (tui, theme, _kb, done) => - new ChainClarifyComponent(tui, theme, chain, done), - { - overlay: true, - overlayOptions: { anchor: "center", width: 84, maxHeight: "80%" }, - }, - ); - } catch (error) { - pi.sendMessage({ - customType: "pi-subagent-result", - content: error instanceof Error ? error.message : String(error), - display: true, - }); - return; - } - if (!result || result.action === "cancel") return; - try { - chain = normalizeAndPreflight(result.steps); - } catch (error) { - pi.sendMessage({ - customType: "pi-subagent-result", - content: error instanceof Error ? error.message : String(error), - display: true, - }); - return; - } - if (result.action === "bg") bg = true; - } - ``` - - Catch only the custom call inside the preview branch. Report synchronous factory throws and rejected custom promises through the existing visible result shape, before final preflight or dispatch. Keep cancellation silent. - -- [ ] **Step 5: Run the integration checks.** - - Run: - - ```bash - mise exec node@24.15.0 -- pnpm vitest run tests/chain-clarify.test.ts tests/slash-chain.test.ts tests/core/chain-clarify-integration.test.ts - ``` - - Expected: CSI-u confirmation reaches the first child, cancellation and both failure forms cannot dispatch, non-TUI/--yes/--bg paths bypass the preview, and existing flag/materialization coverage remains green. - -### Task 4: Verify and commit - -- [ ] **Step 1: Run the full checks.** - - Run: - - ```bash - mise exec node@24.15.0 -- pnpm check - git diff --check - ``` - - If temporary-repository tests inherit a user-level commit.gpgsign setting, rerun the same check with GIT_CONFIG_GLOBAL=/dev/null; do not change repository or global Git configuration. - -- [ ] **Step 2: Perform the terminal-only manual check.** - - In a CSI-u/Kitty-capable terminal, run /run-chain and /chain. Verify arrows, j/k, editing, native backspace/cursor behavior, Escape cancel, reopen, Enter confirmation, and b background action. Record the manual check as unavailable rather than passing when no such terminal is available. - -- [ ] **Step 3: Review the implementation diff.** - - Confirm the implementation changes only src/tui/chain-clarify.ts, src/core/slash-chain.ts, tests/chain-clarify.test.ts, and tests/slash-chain.test.ts. Do not modify tests/core/chain-clarify-integration.test.ts unless a narrowly justified command-boundary regression is added. - -- [ ] **Step 4: Commit the implementation.** - - ```bash - git add src/tui/chain-clarify.ts src/core/slash-chain.ts tests/chain-clarify.test.ts tests/slash-chain.test.ts - git commit -m "fix: support modern chain preview input" - ``` diff --git a/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-3-dashboard-foundation.md b/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-3-dashboard-foundation.md deleted file mode 100644 index 69f6f2f..0000000 --- a/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-3-dashboard-foundation.md +++ /dev/null @@ -1,466 +0,0 @@ -# Phase 3: Dashboard Foundation Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Add one local dashboard renderer and use it for every `ChainClarifyComponent` presentation without changing chain execution or editing behavior. - -**Architecture:** A dependency-free `dashboard-style.ts` owns shared overlay options, an ANSI-safe heavy frame, selected-row viewport fitting, and a bounded small-terminal fallback. `ChainClarifyComponent` remains the single preview/editor used by slash chains and structured subagent clarification; only the slash-chain caller supplies overlay options. - -**Tech Stack:** TypeScript, `@earendil-works/pi-tui@0.84.3`, Vitest, pnpm, Node `24.15.0`. - -**Spec:** `docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui.md#approved-design` - -**Prerequisite:** Phase 2 chain input repair is merged at `3408875`. - -**Usable result:** Sequential, static-parallel, and dynamic-parallel chain previews render inside a responsive pi-status-inspired frame while retaining Phase 2 input, focus, editing, cancellation, and dispatch behavior. - -## Global Constraints - -- Keep the three Pi development packages and lockfile resolved at `0.84.3`. -- Use only Pi's active theme; add no palette, setting, config file, pi-status dependency, or shared package. -- Preserve `/run-chain`, `/chain`, prompt workflows, `--yes`, `--bg`, chain schemas, tool contracts, agent definitions, and settings formats. -- Preserve `ChainClarifyResult = { action: "run" | "cancel" | "bg"; steps: ChainStep[] }`. -- Preserve Phase 2's `matchesKey()`, key-release guard, native `Input`, focus forwarding, cursor positioning, and preview-failure behavior. -- Keep static and dynamic parallel steps read-only. -- Center slash-chain overlays at `92%` width and `85%` maximum height. -- Use Pi/TUI's `truncateToWidth()` and `visibleWidth()` for terminal width calculations. -- Do not replace the renderer with `Box` or `ScrollView`; neither owns this component's selected logical-step mapping or required heavy-frame geometry. - ---- - -### Task 1: Add ANSI-safe dashboard primitives - -**Files:** - -- Create: `src/tui/dashboard-style.ts` -- Create: `tests/dashboard-style.test.ts` - -**Interfaces:** - -- Consumes: `OverlayOptions`, `truncateToWidth()`, and `visibleWidth()` from `@earendil-works/pi-tui`; `Theme` from `src/tui/agent-widget.ts`. -- Produces: - - ```ts - export const DASHBOARD_MAX_HEIGHT_RATIO = 0.85; - export const DASHBOARD_OVERLAY_OPTIONS: OverlayOptions; - export const MIN_DASHBOARD_FRAME_WIDTH = 7; - export function dashboardContentWidth(width: number): number; - export function renderDashboardFrame( - lines: readonly string[], - width: number, - theme: Theme, - ): string[]; - export function fitDashboardViewport( - lines: readonly string[], - selectedLine: number | undefined, - height: number, - offset: number, - ): { lines: string[]; offset: number }; - export function renderDashboardTooSmall( - width: number, - height: number, - theme: Theme, - ): string[]; - ``` - -- [ ] **Step 1: Write the failing primitive tests.** - - Create `tests/dashboard-style.test.ts` with a plain theme and an ANSI theme. Cover these exact contracts: - - ```ts - import { visibleWidth } from "@earendil-works/pi-tui"; - import { describe, expect, test } from "vitest"; - import { - dashboardContentWidth, - fitDashboardViewport, - MIN_DASHBOARD_FRAME_WIDTH, - renderDashboardFrame, - renderDashboardTooSmall, - } from "../src/tui/dashboard-style.js"; - - const plainTheme = { - fg: (_color: string, text: string) => text, - bold: (text: string) => text, - }; - const ansiTheme = { - fg: (_color: string, text: string) => `\x1b[31m${text}\x1b[39m`, - bold: (text: string) => `\x1b[1m${text}\x1b[22m`, - }; - - test("renders a heavy frame at the requested visible width", () => { - const lines = renderDashboardFrame(["Header", "Body"], 24, plainTheme); - expect(lines).toHaveLength(6); - expect(lines[0]).toBe(`┏${"━".repeat(22)}┓`); - expect(lines.at(-1)).toBe(`┗${"━".repeat(22)}┛`); - expect(lines.every((line) => visibleWidth(line) === 24)).toBe(true); - expect(dashboardContentWidth(24)).toBe(18); - }); - - test("preserves visible width with ANSI and embedded newlines", () => { - const lines = renderDashboardFrame(["one\r\ntwo"], 24, ansiTheme); - expect(lines.some((line) => line.includes("\x1b[31m"))).toBe(true); - expect(lines.every((line) => visibleWidth(line) === 24)).toBe(true); - expect(lines.every((line) => !/[\r\n]/.test(line))).toBe(true); - }); - - test("preserves the complete seven-column minimum", () => { - expect(MIN_DASHBOARD_FRAME_WIDTH).toBe(7); - expect(renderDashboardFrame(["x"], 7, plainTheme)).toEqual([ - "┏━━━━━┓", - "┃ ┃", - "┃ x ┃", - "┃ ┃", - "┗━━━━━┛", - ]); - }); - - test("keeps selection visible without unnecessary offset jumps", () => { - const lines = ["0", "1", "2", "3", "4"]; - expect(fitDashboardViewport(lines, 4, 3, 0)).toEqual({ - lines: ["2", "3", "4"], - offset: 2, - }); - expect(fitDashboardViewport(lines, 2, 3, 1)).toEqual({ - lines: ["1", "2", "3"], - offset: 1, - }); - expect(fitDashboardViewport(["0", "1"], 1, 3, 99)).toEqual({ - lines: ["0", "1", ""], - offset: 0, - }); - expect(fitDashboardViewport(lines, 2, 0, 1)).toEqual({ - lines: [], - offset: 0, - }); - }); - - test("renders a bounded small-terminal escape message", () => { - const lines = renderDashboardTooSmall(30, 3, plainTheme); - expect(lines).toHaveLength(3); - expect(lines.every((line) => visibleWidth(line) === 30)).toBe(true); - expect(lines.join("\n")).toContain("Esc"); - }); - ``` - -- [ ] **Step 2: Run the tests and confirm the missing-module failure.** - - ```bash - pnpm vitest run tests/dashboard-style.test.ts - ``` - - Expected: FAIL because `src/tui/dashboard-style.ts` does not exist. - -- [ ] **Step 3: Implement the minimal shared renderer.** - - In `src/tui/dashboard-style.ts`, use two columns of horizontal padding and these exact fixed values: - - ```ts - const PADDING_X = 2; - const FRAME = { tl: "┏", tr: "┓", bl: "┗", br: "┛", h: "━", v: "┃" } as const; - - export const DASHBOARD_MAX_HEIGHT_RATIO = 0.85; - export const MIN_DASHBOARD_FRAME_WIDTH = 7; - export const DASHBOARD_OVERLAY_OPTIONS: OverlayOptions = { - anchor: "center", - width: "92%", - maxHeight: "85%", - }; - ``` - - `dashboardContentWidth(width)` returns `max(1, floor(width) - 6)`. The internal padding helper replaces embedded CR/LF runs with one space, truncates with an empty ellipsis, and pads using `visibleWidth()`. - - `renderDashboardFrame()` emits the top border, one blank row, all content rows, one blank row, and the bottom border. Color frame glyphs with `theme.fg("border", glyph)` and clamp every row to the requested visible width. - - `fitDashboardViewport()` follows the pi-status algorithm: clamp height and offset, move only when the selected line is above or below the visible range, slice the visible lines, and pad short content with empty rows. - - `renderDashboardTooSmall()` clamps dimensions to at least `1`, returns exactly that many fixed-width rows, and places `theme.fg("accent", "Terminal too small · Esc")` on the middle row. - -- [ ] **Step 4: Run the primitive tests and typecheck.** - - ```bash - pnpm vitest run tests/dashboard-style.test.ts - pnpm typecheck - git diff --check - ``` - - Expected: all primitive tests pass and every rendered row has the requested ANSI-visible width. - -- [ ] **Step 5: Commit the reusable foundation.** - - ```bash - git add src/tui/dashboard-style.ts tests/dashboard-style.test.ts - git commit -m "feat: add dashboard ui primitives" - ``` - ---- - -### Task 2: Render the shared chain preview and editor - -**Files:** - -- Modify: `src/tui/chain-clarify.ts` -- Modify: `tests/chain-clarify.test.ts` - -**Interfaces:** - -- Consumes: all exports from `src/tui/dashboard-style.ts` except `DASHBOARD_OVERLAY_OPTIONS`. -- Preserves: the `ChainClarifyComponent` constructor, `Component`/`Focusable` behavior, `ChainClarifyResult`, native `Input`, and all Phase 2 keys and callbacks. -- Produces: a framed list/editor bounded by `tui.terminal.rows` whose selected logical step remains visible. - -- [ ] **Step 1: Give each test component a configurable terminal height.** - - Replace the shared partial `mockTui` with a `makeComponent()`-local TUI containing: - - ```ts - const tui = { - requestRender, - terminal: { columns: 80, rows: terminalRows }, - } as unknown as TUI; - ``` - - Add `terminalRows = 40` as the final `makeComponent()` parameter so resize cases do not mutate shared test state. - -- [ ] **Step 2: Add failing dashboard render regressions.** - - Keep all current tests and add cases that assert: - - ```ts - test("renders a framed preview with footer and visible selection", () => { - const { component } = makeComponent([ - { agent: "scout", task: "analyze" }, - { agent: "worker", task: "change" }, - ]); - const lines = component.render(80); - expect(lines[0]).toContain("┏"); - expect(lines.at(-1)).toContain("┗"); - expect(lines.find((line) => line.includes("scout"))).toContain("▸"); - expect(lines.join("\n")).toContain("Enter Run"); - expect(lines.every((line) => visibleWidth(line) === 80)).toBe(true); - }); - - test("scrolls the complete selected sequential step into view", () => { - const steps = Array.from({ length: 8 }, (_, index) => ({ - agent: `agent-${index}`, - task: `task-${index}`, - model: `model-${index}`, - })); - const { component } = makeComponent(steps, undefined, 13); - for (let index = 1; index < steps.length; index++) - component.handleInput("j"); - const output = component.render(80).join("\n"); - expect(output).toContain("▸ [8/8] agent-7"); - expect(output).toContain("task-7"); - expect(output).toContain("model-7"); - expect(output).not.toContain("agent-0"); - }); - - test("renders bounded width and height fallbacks", () => { - expect( - makeComponent(undefined, undefined, 10).component.render(30).join("\n"), - ).toContain("Esc"); - expect(makeComponent().component.render(6).join("\n")).not.toContain("┏"); - }); - - test("renders static and dynamic parallel agents read-only", () => { - const { component } = makeComponent([ - { parallel: [{ agent: "scout" }, { agent: "worker" }] }, - { - expand: { from: { output: "items", path: "$.items" } }, - parallel: { agent: "reviewer", task: "review {{item}}" }, - collect: { as: "reviews" }, - }, - ]); - const output = component.render(100).join("\n"); - expect(output).toContain("Parallel · scout, worker"); - expect(output).toContain("Dynamic parallel · reviewer"); - component.handleInput("e"); - expect(component.render(100).join("\n")).not.toContain("Edit Task"); - }); - ``` - - Also assert task/model edit modes retain the heavy frame and native input, and a task containing `"first\nsecond"` produces no embedded CR/LF in any returned row. Update existing marker assertions from `>` to `▸`. - -- [ ] **Step 3: Run the component test and confirm only new layout assertions fail.** - - ```bash - pnpm vitest run tests/chain-clarify.test.ts - ``` - - Expected: Phase 2 interaction tests pass; frame, viewport, marker, parallel-label, and fallback assertions fail. - -- [ ] **Step 4: Build logical list rows and selected-line metadata.** - - Add a component-level `viewportOffset = 0`. Render sequential steps as three-row blocks: - - ```text - ▸ [1/N] scout - Task analyze - Model (inherit) - ``` - - Preserve the existing `* ` prefix for task/model overrides. Separate logical steps with one blank row between steps, with no trailing separator. For the selected sequential step, pass the model row index to `fitDashboardViewport()` so a three-row viewport contains its marker, task, and model together. - - Render static parallel groups as one row: - - ```text - ▸ [2/N] Parallel · scout, worker - ``` - - Render dynamic parallel groups as one row: - - ```text - ▸ [3/N] Dynamic parallel · reviewer - ``` - - Discriminate dynamic groups first with `"expand" in step`; otherwise an object with `"parallel" in step` is the static group whose `parallel` value is an array. Both variants ignore `e` and `m`, preserving read-only behavior. - -- [ ] **Step 5: Apply the chain-specific height budget.** - - Keep these values private to `chain-clarify.ts`: - - ```ts - const DASHBOARD_CHROME_ROWS = 8; - const MIN_CHAIN_DASHBOARD_ROWS = 11; - ``` - - Calculate: - - ```ts - const maxRows = Math.max( - 1, - Math.floor(this.tui.terminal.rows * DASHBOARD_MAX_HEIGHT_RATIO), - ); - const targetRows = - maxRows < MIN_CHAIN_DASHBOARD_ROWS - ? maxRows - : Math.min( - maxRows, - DASHBOARD_CHROME_ROWS + Math.max(3, logicalBody.lines.length), - ); - ``` - - Return `renderDashboardTooSmall(width, targetRows, theme)` when `width < 7` or `targetRows < 11`. Otherwise give `targetRows - 8` rows to `fitDashboardViewport()`, store its returned offset, and frame: - - ```text - Chain Preview · N steps - - - - ↑/↓ Select • e Edit task • m Edit model • Enter Run • b Background • q/Esc Cancel - ``` - -- [ ] **Step 6: Frame native edit mode without changing input behavior.** - - Render `Edit Task` or `Edit Model`, a three-row viewport containing `input.render(dashboardContentWidth(width))`, and `Enter Submit • Esc Cancel` through the same height/fallback/frame path. Do not replace the native input or change its submit, escape, focus, or disposal callbacks. - -- [ ] **Step 7: Run component and primitive verification.** - - ```bash - pnpm vitest run tests/dashboard-style.test.ts tests/chain-clarify.test.ts - pnpm typecheck - git diff --check - ``` - - Expected: all existing input/edit tests and all new responsive-layout tests pass. - -- [ ] **Step 8: Commit the chain renderer migration.** - - ```bash - git add src/tui/chain-clarify.ts tests/chain-clarify.test.ts - git commit -m "feat: render chain preview as dashboard" - ``` - ---- - -### Task 3: Use shared slash-chain overlay options - -**Files:** - -- Modify: `src/core/slash-chain.ts` -- Modify: `tests/slash-chain.test.ts` - -**Interfaces:** - -- Consumes: `DASHBOARD_OVERLAY_OPTIONS` from `src/tui/dashboard-style.ts`. -- Preserves: the Phase 2 condition `ctx.mode === "tui" && !bg && !yes`, cancellation, rejected-preview reporting, and manager dispatch. -- Leaves unchanged: `src/core/subagent.ts`; its structured clarification path shares the component renderer but remains a non-overlay custom UI with the same result contract. - -- [ ] **Step 1: Update the existing overlay-options regression.** - - In `confirms a TUI preview through CSI-u input`, replace the literal expectation with: - - ```ts - expect(seenOptions).toEqual({ - overlay: true, - overlayOptions: { - anchor: "center", - width: "92%", - maxHeight: "85%", - }, - }); - ``` - -- [ ] **Step 2: Run the slash-chain test and confirm the old dimensions fail.** - - ```bash - pnpm vitest run tests/slash-chain.test.ts - ``` - - Expected: the overlay-options assertion receives `{ width: 84, maxHeight: "80%" }`; confirmation, cancellation, rejection, RPC, `--yes`, and `--bg` tests remain green. - -- [ ] **Step 3: Replace literal dimensions with the shared constant.** - - Import `DASHBOARD_OVERLAY_OPTIONS` and pass: - - ```ts - { - overlay: true, - overlayOptions: DASHBOARD_OVERLAY_OPTIONS, - } - ``` - - Do not change the preview condition, error boundary, normalization, or dispatch flow. - -- [ ] **Step 4: Run focused verification.** - - ```bash - pnpm vitest run \ - tests/dashboard-style.test.ts \ - tests/chain-clarify.test.ts \ - tests/slash-chain.test.ts \ - tests/core/chain-clarify-integration.test.ts - pnpm typecheck - git diff --check - ``` - - Expected: all focused tests pass and production changes are limited to the renderer, chain presentation, and shared slash-chain overlay options. - -- [ ] **Step 5: Run the full repository check.** - - In a sandbox where inherited Git signing blocks temporary test repositories, run: - - ```bash - env GIT_CONFIG_COUNT=1 \ - GIT_CONFIG_KEY_0=commit.gpgsign \ - GIT_CONFIG_VALUE_0=false \ - pnpm check - ``` - - Otherwise run `pnpm check` normally. Expected: 56 existing test files and 1,293 existing tests plus the new Phase 3 tests pass; existing non-fatal Biome warnings may remain unchanged. - -- [ ] **Step 6: Perform interactive TUI verification.** - - Open `/run-chain` with one sequential step, a long sequential chain, a static parallel group, and a dynamic parallel group. At normal and undersized terminal dimensions verify selection, task/model editing, run, background, cancel, resize, and fallback behavior. If no interactive TUI is available, record that manual visual verification was not run. - -- [ ] **Step 7: Commit the overlay wiring.** - - ```bash - git add src/core/slash-chain.ts tests/slash-chain.test.ts - git commit -m "feat: use dashboard chain overlay" - ``` - -## Out of Scope - -- Do not modify `tests/core/chain-clarify-integration.test.ts`; Phase 2 already placed command-to-component coverage in `tests/slash-chain.test.ts`. -- Do not alter the structured subagent clarification call site, chain preflight, execution, managers, widgets, `/agents`, fleet, inline renderers, or conversation viewer. -- Do not add height-budget exports for later phases; each later surface has different fixed chrome and can consume the shared ratio, frame, viewport, and fallback directly. diff --git a/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-4-live-widgets.md b/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-4-live-widgets.md deleted file mode 100644 index cfcbbe9..0000000 --- a/docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui-phase-4-live-widgets.md +++ /dev/null @@ -1,497 +0,0 @@ -# Phase 4: Live Widgets Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Restyle the persistent AgentWidget and ChainWidget with the dashboard hierarchy while preserving placement and lifecycle, honoring Pi's component width, and bounding each widget to 12 rows. - -**Architecture:** Keep both widgets' existing state selection, timers, registration, invalidation, and cleanup. Change only width-aware render composition: an open dashboard heading, prefixed body rows, and a summary footer; reuse Pi/TUI truncation and the existing viewport helper instead of adding another renderer. - -**Tech Stack:** TypeScript, `@earendil-works/pi-tui@0.84.3`, Vitest, pnpm, Node `24.15.0`. - -**Spec:** `docs/superpowers/plans/2026-08-26-run-chain-dashboard-ui.md#approved-design` - -**Prerequisite:** Phase 3 dashboard foundation is merged at `23ea3a2`. - -**Usable result:** Running agents and chains display a compact, width-safe dashboard above the editor without flooding the terminal. - -## Global Constraints - -- Keep AgentWidget and ChainWidget above the editor under their existing `agents` and `chain` widget keys. -- Preserve manager state, filtering, status text, timers, spinner cadence, task ordering, completion behavior, subscriptions, invalidation, and cleanup. -- Preserve AgentWidget's 12-row maximum; apply the same 12-row maximum to ChainWidget because Pi does not cap custom component widgets. -- Use the width passed to `Component.render(width)`, not `tui.terminal.columns`. -- Normalize embedded CR/LF runs, truncate with `truncateToWidth(text, width, "")`, and assert widths with `visibleWidth()`. -- Use only the active Pi theme roles already used by the widgets: `accent`, `success`, `warning`, `error`, `muted`, and `dim`. -- Do not add a dependency, shared widget abstraction, palette, setting, frame component, or pi-status runtime import. -- Keep `renderFinishedLine()` and `statusIcon()` status semantics unchanged. - -## Reference Evidence - -- Pi `v0.84.3`: `packages/coding-agent/src/core/extensions/types.ts` defines component widgets as `render(width)`, while `interactive-mode.ts` caps only string-array widgets. -- pi-status: `src/tui/overlay-render.ts` and `src/tui/dashboard-layout.ts` establish ANSI-visible truncation, CR/LF normalization, and selected-row-aware viewport fitting. Reuse the local Phase 3 equivalent; do not port pi-status themes or state. -- nicobailon-pi-subagents: `src/tui/fleet-status.ts` renders from the supplied component width and bounds the live roster explicitly. -- tintinweb-pi-subagents: `src/ui/agent-widget.ts` is the closest rendering ancestor, and `test/agent-widget.test.ts` demonstrates that queue visibility and overflow accounting need dedicated regressions. Do not port its unrelated lifecycle or color features. - ---- - -### Task 1: Make AgentWidget width-aware and restyle it - -**Files:** - -- Modify: `src/tui/agent-widget.ts` -- Test: `tests/agent-widget.test.ts` - -**Interfaces:** - -- Change the local `UICtx.setWidget()` component contract from `render(): string[]` to `render(width: number): string[]`. -- Change private rendering to `renderWidget(width: number, theme: Theme): string[]`. -- Preserve the public `AgentWidget` constructor and methods. -- Produce at most 12 rows: one heading, up to 10 body rows, and one summary footer. - -- [ ] **Step 1: Update the test capture helper to exercise Pi's render width.** - - Import `visibleWidth` and change `captureRender()` so the mock terminal width can disagree with the allocated component width: - - ```ts - function captureRender( - ctx: ReturnType, - theme: Theme, - width = 200, - terminalColumns = 200, - ): string[] { - const calls = ctx.setWidget.mock.calls as Array< - [ - string, - ( - | (( - tui: unknown, - theme: Theme, - ) => { - render(width: number): string[]; - }) - | undefined - ), - unknown, - ] - >; - const factory = calls.find(([key]) => key === "agents")?.[1]; - if (!factory) return []; - const tui = { - terminal: { columns: terminalColumns }, - requestRender: vi.fn(), - }; - return factory(tui, theme).render(width); - } - ``` - -- [ ] **Step 2: Add failing dashboard grammar and summary tests without removing lifecycle coverage.** - - Keep every existing test. Add focused cases that lock these contracts: - - ```ts - it("renders the live-agent dashboard hierarchy", () => { - const manager = makeMockManager([ - makeRecord({ - type: "worker", - description: "implement phase four", - status: "running", - turnCount: 2, - live: { activeTools: ["read"], responseText: "" }, - }), - makeRecord({ id: "queued", status: "queued" }), - ]); - const ctx = makeMockUICtx(); - const widget = new AgentWidget(manager); - widget.setUICtx(ctx); - widget.update(); - - const lines = captureRender(ctx, makeMockTheme(), 100); - expect(lines[0]).toBe("╭─ ✦ AGENTS"); - expect(lines[1]).toMatch(/^│ [⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏] worker implement phase four · /); - expect(lines[2]).toContain("│ ⎿ reading…"); - expect(lines.at(-1)).toBe("╰─ 1 running · 1 queued"); - widget.dispose(); - }); - - it("keeps queue totals visible when active rows fill the widget", () => { - const agents = [ - ...Array.from({ length: 7 }, (_, index) => - makeRecord({ id: `run-${index}`, status: "running" }), - ), - ...Array.from({ length: 3 }, (_, index) => - makeRecord({ id: `queue-${index}`, status: "queued" }), - ), - ]; - const ctx = makeMockUICtx(); - const widget = new AgentWidget(makeMockManager(agents)); - widget.setUICtx(ctx); - widget.update(); - - const lines = captureRender(ctx, makeMockTheme(), 100); - expect(lines).toHaveLength(12); - expect(lines.at(-1)).toBe("╰─ 7 running · 3 queued · +2 more"); - widget.dispose(); - }); - - it("renders a finished-only dashboard", () => { - const agent = makeRecord({ - id: "finished", - status: "completed", - completedAt: Date.now(), - }); - const ctx = makeMockUICtx(); - const widget = new AgentWidget(makeMockManager([agent])); - widget.markFinished(agent.id); - widget.setUICtx(ctx); - widget.update(); - - const lines = captureRender(ctx, makeMockTheme(), 100); - expect(lines[0]).toBe("╭─ ✦ AGENTS"); - expect(lines[1]).toMatch(/^│ ✓ scout do the thing · /); - expect(lines.at(-1)).toBe("╰─ 1 finished"); - widget.dispose(); - }); - ``` - -- [ ] **Step 3: Add failing allocated-width and dynamic-text safety tests.** - - Use an ANSI theme and widths that are smaller than `tui.terminal.columns`: - - ```ts - it("uses the allocated width and keeps every row single-line", () => { - const ansiTheme: Theme = { - fg: (_color, text) => `\x1b[31m${text}\x1b[39m`, - bold: (text) => `\x1b[1m${text}\x1b[22m`, - }; - const agent = makeRecord({ - description: "a long description\r\nwith a second line", - status: "running", - }); - const ctx = makeMockUICtx(); - const widget = new AgentWidget(makeMockManager([agent])); - widget.setUICtx(ctx); - widget.update(); - - for (const width of [0, 1, 12, 40]) { - const lines = captureRender(ctx, ansiTheme, width, 200); - expect(lines.every((line) => visibleWidth(line) <= width)).toBe(true); - expect(lines.every((line) => !/[\r\n]/.test(line))).toBe(true); - } - widget.dispose(); - }); - ``` - - Expected failure before implementation: rows use the 200-column terminal width, the old heading is rendered, and no footer summary exists. - -- [ ] **Step 4: Run the AgentWidget tests and confirm presentation-only failures.** - - Run: - - ```bash - pnpm vitest run tests/agent-widget.test.ts - ``` - - Expected: the new grammar, footer, width, and row-cap assertions fail; existing lifecycle, filtering, status, and linger assertions pass. - -- [ ] **Step 5: Implement the minimum width-aware composition.** - - In `src/tui/agent-widget.ts`: - 1. Keep `MAX_WIDGET_LINES = 12`; derive a 10-row body budget by reserving one heading and one footer row. - 2. Make the widget component callback `render: (width) => this.renderWidget(width, theme)`. - 3. Inside `renderWidget()`, clamp width with `Math.max(0, Math.floor(width))` and use one local closure that replaces CR/LF runs and calls `truncateToWidth(line, safeWidth, "")`. - 4. Render the heading as `╭─ ✦ AGENTS` using `accent` while active and `dim` when finished-only. - 5. Render each running agent as an atomic two-row unit: - - ```text - │ · - │ ⎿ - ``` - - 6. Render each visible finished agent as one `│ ` row using the unchanged `renderFinishedLine()` output. - 7. Do not render a separate queued body row. Put all non-zero totals in the footer in `running`, `queued`, `finished` order, using existing total counts rather than visible-row counts. - 8. Fill the body in the existing priority order: complete running pairs first, then finished rows. Never split a running pair. Append `+N more` to the footer when running or finished agents were omitted. - 9. Truncate heading, body, and footer with the same local width closure before returning them. - - Do not change `update()`, `onTurnStart()`, `markFinished()`, timers, status text, registration, invalidation, or disposal. - -- [ ] **Step 6: Run focused verification.** - - Run: - - ```bash - pnpm vitest run tests/agent-widget.test.ts - pnpm typecheck - git diff --check - ``` - - Expected: all AgentWidget tests pass, the component typechecks with `render(width)`, and the diff has no whitespace errors. - -- [ ] **Step 7: Commit the AgentWidget change.** - - ```bash - git add src/tui/agent-widget.ts tests/agent-widget.test.ts - git commit -m "feat: restyle live agent widget" - ``` - ---- - -### Task 2: Bound and restyle ChainWidget - -**Files:** - -- Modify: `src/tui/chain-widget.ts` -- Test: `tests/chain-widget.test.ts` - -**Interfaces:** - -- Consume `fitDashboardViewport()` from `src/tui/dashboard-style.ts`. -- Change `renderLines()` to `renderLines(snapshot: WorkflowGraphSnapshot, theme: Theme, width: number): string[]`. -- Keep `update()`, `clear()`, `dispose()`, `statusIcon()`, snapshot types, widget key, and placement unchanged. -- Produce at most 12 rows: one heading, up to 10 selected-aware body rows, and one summary footer. - -- [ ] **Step 1: Add a helper that renders the registered component at an explicit width.** - - Extend the test context so it retains the factory, then render it with a terminal width that differs from the component width: - - ```ts - function renderRegisteredWidget( - ctx: ReturnType, - width: number, - terminalColumns = 200, - ): string[] { - const factory = ctx.widgets.get("chain") as - | (( - tui: unknown, - theme: Theme, - ) => { - render(width: number): string[]; - }) - | undefined; - if (!factory) return []; - return factory( - { terminal: { columns: terminalColumns }, requestRender() {} }, - mockTheme(), - ).render(width); - } - ``` - -- [ ] **Step 2: Replace loose render assertions with failing exact hierarchy tests.** - - Keep lifecycle tests and status/error coverage. Update direct `renderLines()` calls to pass a width and add: - - ```ts - test("renders sequential progress with a dashboard summary", () => { - const widget = new ChainWidget(); - const lines = widget.renderLines( - makeSnapshot({ - nodes: [ - { - id: "step-0", - kind: "step", - label: "Scan files", - status: "completed", - flatIndex: 0, - stepIndex: 0, - }, - { - id: "step-1", - kind: "step", - label: "Create plan", - status: "running", - flatIndex: 1, - stepIndex: 1, - }, - { - id: "step-2", - kind: "step", - label: "Implement", - status: "pending", - flatIndex: 2, - stepIndex: 2, - }, - ], - currentNodeId: "step-1", - }), - mockTheme(), - 100, - ); - - expect(lines[0]).toBe("╭─ ✦ CHAIN · test-chain"); - expect(lines[1]).toBe("│ [1/3] ✓ Scan files"); - expect(lines[2]).toMatch(/^│ \[2\/3\] [⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏] Create plan$/); - expect(lines[3]).toBe("│ [3/3] ○ Implement"); - expect(lines.at(-1)).toBe("╰─ 1 completed · 1 running · 1 pending"); - }); - - const parallelSnapshot = makeSnapshot({ - nodes: [ - { - id: "step-0", - kind: "parallel-group", - label: "Parallel group (2)", - status: "running", - stepIndex: 0, - children: [ - { - id: "step-0-agent-0", - kind: "agent", - label: "Worker A", - status: "completed", - flatIndex: 0, - stepIndex: 0, - }, - { - id: "step-0-agent-1", - kind: "agent", - label: "Worker B", - status: "running", - flatIndex: 1, - stepIndex: 0, - }, - ], - }, - ], - currentNodeId: "step-0-agent-1", - }); - - test("keeps parallel children under their parent row", () => { - const lines = new ChainWidget().renderLines( - parallelSnapshot, - mockTheme(), - 100, - ); - expect( - lines.some( - (line) => line.startsWith("│ [1/1]") && line.includes("Parallel group"), - ), - ).toBe(true); - expect( - lines.some( - (line) => line.startsWith("│ ├─") && line.includes("Worker A"), - ), - ).toBe(true); - expect( - lines.some( - (line) => line.startsWith("│ └─") && line.includes("Worker B"), - ), - ).toBe(true); - }); - ``` - -- [ ] **Step 3: Add failing row-cap, active-row visibility, and width tests.** - - ```ts - const longLabelSnapshot = makeSnapshot({ - nodes: [ - { - id: "step-0", - kind: "step", - label: "long label\r\nsecond line", - status: "running", - flatIndex: 0, - stepIndex: 0, - }, - ], - currentNodeId: "step-0", - }); - - test("caps output while keeping the active row visible", () => { - const nodes = Array.from({ length: 15 }, (_, index) => ({ - id: `step-${index}`, - kind: "step" as const, - label: `Step ${index}`, - status: - index < 12 - ? ("completed" as const) - : index === 12 - ? ("running" as const) - : ("pending" as const), - flatIndex: index, - stepIndex: index, - })); - const lines = new ChainWidget().renderLines( - makeSnapshot({ nodes, currentNodeId: "step-12" }), - mockTheme(), - 60, - ); - - expect(lines).toHaveLength(12); - expect(lines.join("\n")).toContain("Step 12"); - expect(lines.join("\n")).not.toContain("Step 0"); - expect(lines.at(-1)).toContain("+5 more"); - }); - - test("uses the component width rather than terminal columns", () => { - const widget = new ChainWidget(); - const ctx = mockUICtx(); - widget.setUICtx(ctx); - widget.update(longLabelSnapshot); - - for (const width of [0, 1, 12, 40]) { - const lines = renderRegisteredWidget(ctx, width, 200); - expect(lines.every((line) => visibleWidth(line) <= width)).toBe(true); - expect(lines.every((line) => !/[\r\n]/.test(line))).toBe(true); - } - widget.dispose(); - }); - - test("measures ANSI output by visible width", () => { - const ansiTheme: Theme = { - fg: (_color, text) => `\x1b[31m${text}\x1b[39m`, - bold: (text) => `\x1b[1m${text}\x1b[22m`, - }; - const lines = new ChainWidget().renderLines( - longLabelSnapshot, - ansiTheme, - 40, - ); - expect(lines.every((line) => visibleWidth(line) <= 40)).toBe(true); - expect(lines.every((line) => !/[\r\n]/.test(line))).toBe(true); - }); - ``` - - Expected failure before implementation: ChainWidget has no heading/footer grammar, active-row viewport, or component-width rendering. - -- [ ] **Step 4: Run the ChainWidget tests and confirm presentation-only failures.** - - Run: - - ```bash - pnpm vitest run tests/chain-widget.test.ts - ``` - - Expected: the new hierarchy, cap, viewport, and width assertions fail; registration, clearing, empty-snapshot, parallel, phase, and error behavior remains green. - -- [ ] **Step 5: Implement the bounded width-aware composition.** - - In `src/tui/chain-widget.ts`: - 1. Add a local `MAX_WIDGET_LINES = 12` and derive a 10-row body budget. - 2. Build body rows as `{ nodeId: string; text: string }` so `snapshot.currentNodeId` maps to the row that must remain visible. - 3. Render top-level rows as `│ [index/total]