diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 9a358b46f..d38411b3c 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -9796,6 +9796,35 @@ function createGentleAiExtensionForTesting( reminderEpoch += 1; unbindPreparation?.(); reminderManager = ctx.sessionManager; + // gentle-shell#1690: a delegated child runs in the parent's resolved + // worktree. Repository preparation, review negotiation, asset install and + // model config belong to the parent session and write shared state. The + // standing review grant is host-only (a child never captures an identity), + // so revoke/refresh have nothing to act on; the child relay is load-time. + // Only that parent-owned work is skipped: the session-local resets above, + // the dev-binary notice, and any step added after this call, still run in + // children. + if (permissionEnvironment.GENTLE_PI_AGENTS_CHILD !== "1") await startParentSession(event, ctx); + else await surfaceDevBinaryOverride(ctx); + }); + + // Loud, every session: an active dev-binary override means this session + // runs an unpinned gentle-ai. One visible startup notice: the gentle-shell + // 🌹 card owns the announcement when it can render (shell enabled with UI); + // this toast is only the fallback for when the card is unavailable. The + // hasUI guard stays: headless contexts have no toast to show. + const surfaceDevBinaryOverride = async (ctx: ExtensionContext): Promise => { + const devBinaryToastFallback = ctx.hasUI && !shellEnabled(); + try { + const devBinary = await describeDevBinaryOverride(); + if (devBinaryToastFallback && devBinary.state === "active") ctx.ui.notify(devBinary.line, "warning"); + if (devBinaryToastFallback && devBinary.state === "invalid") ctx.ui.notify(devBinary.line, "error"); + } catch (error) { + if (ctx.hasUI) ctx.ui.notify(`Gentle AI dev binary override check failed: ${error instanceof Error ? error.message : String(error)}`, "warning"); + } + }; + + const startParentSession = async (event: unknown, ctx: ExtensionContext): Promise => { const epoch = reminderEpoch; const manager = ctx.sessionManager; const originalCwd = manager?.getCwd?.() ?? ctx.cwd; @@ -9816,19 +9845,7 @@ function createGentleAiExtensionForTesting( const reason = (event as { reason?: unknown }).reason; if (reason !== "reload") revokeCurrentReviewSessionPermission(ctx); await refreshReviewSessionPermissionStatus(ctx); - // Loud, every session: an active dev-binary override means this session - // runs an unpinned gentle-ai. One visible startup notice: the gentle-shell - // 🌹 card owns the announcement when it can render (shell enabled with UI); - // this toast is only the fallback for when the card is unavailable. The - // hasUI guard stays: headless contexts have no toast to show. - const devBinaryToastFallback = ctx.hasUI && !shellEnabled(); - try { - const devBinary = await describeDevBinaryOverride(); - if (devBinaryToastFallback && devBinary.state === "active") ctx.ui.notify(devBinary.line, "warning"); - if (devBinaryToastFallback && devBinary.state === "invalid") ctx.ui.notify(devBinary.line, "error"); - } catch (error) { - if (ctx.hasUI) ctx.ui.notify(`Gentle AI dev binary override check failed: ${error instanceof Error ? error.message : String(error)}`, "warning"); - } + await surfaceDevBinaryOverride(ctx); try { const installResult = installPackageAssets(ctx.cwd, true, ["delegation", "review"]); migrateLegacyProjectModelOverrides(ctx.cwd); @@ -9864,7 +9881,7 @@ function createGentleAiExtensionForTesting( } catch { // Startup negotiation is best-effort only; never surface or throw. } - }); + }; pi.on("before_agent_start", async (event, ctx) => { const isNamedAgent = isNamedAgentStartEvent(event); @@ -9984,7 +10001,7 @@ function createGentleAiExtensionForTesting( // Persist the observed own write before any await. Preparation is not // mutation evidence, and cannot invent a pre-write Changes baseline. if (root) recordReviewMutation(pi, ctx.sessionManager, root, { source: "direct", toolName: event.toolName, toolCallId: event.toolCallId, ...directWriterProfile(pi, ctx) }); - if (prospectiveRoot && !resolveSessionWorktree(ctx.cwd, ctx.cwd)) await prepareBoundSessionRepository(ctx.sessionManager, ctx.sessionManager.getCwd?.() ?? ctx.cwd, ctx.signal); + if (permissionEnvironment.GENTLE_PI_AGENTS_CHILD !== "1" && prospectiveRoot && !resolveSessionWorktree(ctx.cwd, ctx.cwd)) await prepareBoundSessionRepository(ctx.sessionManager, ctx.sessionManager.getCwd?.() ?? ctx.cwd, ctx.signal); } catch { /* Preparation and receipt persistence cannot change a successful tool result. */ } }); diff --git a/extensions/history/index.ts b/extensions/history/index.ts index 66b1814a1..1a8365603 100644 --- a/extensions/history/index.ts +++ b/extensions/history/index.ts @@ -1316,8 +1316,10 @@ export default function promptHistoryExtension( ): void { const env = deps.env ?? process.env; const configHome = deps.gentlePiConfigHome ?? gentlePiConfigHome(env); - // Per-prompt gate: re-read so a Customize toggle applies live. - const capturing = () => captureEnabled(env, configHome); + // Per-prompt gate: re-read so a Customize toggle applies live. A delegated + // child never captures: its prompt is a delegation brief, not user history, + // and the parent owns the store's init and GC (gentle-shell#1690). + const capturing = () => env.GENTLE_PI_AGENTS_CHILD !== "1" && captureEnabled(env, configHome); const root = deps.root ?? PI_HISTORY_ROOT; const cwd = deps.cwd ?? process.cwd(); const instanceId = deps.instanceId ?? randomUUID(); diff --git a/extensions/pi-pretty.ts b/extensions/pi-pretty.ts index 741df6c19..7457a66ae 100644 --- a/extensions/pi-pretty.ts +++ b/extensions/pi-pretty.ts @@ -32,6 +32,9 @@ export default async function gentlePiPrettyExtension( bundled?: PiPrettyExtension, env: NodeJS.ProcessEnv = process.env, ): Promise { + // gentle-shell#1690: a delegated child has no transcript to prettify, and + // the upstream fallback would start its own file indexing in every child. + if (env.GENTLE_PI_AGENTS_CHILD === "1") return undefined; if (quietToolsEnabled()) { process.env.PRETTY_DISABLE_TOOLS = mergeDisabledTools( process.env.PRETTY_DISABLE_TOOLS, diff --git a/extensions/skill-registry.ts b/extensions/skill-registry.ts index 78fd0997b..304b43d25 100644 --- a/extensions/skill-registry.ts +++ b/extensions/skill-registry.ts @@ -505,6 +505,9 @@ function shouldSkipSkillRegistryStartup( env = process.env, ): boolean { return ( + // gentle-shell#1690: delegated children share the parent's cwd; the + // parent owns .atl/ writes, the legacy rename and the watcher. + env.GENTLE_PI_AGENTS_CHILD === "1" || pi.getFlag(NO_SKILL_REGISTRY_FLAG) === true || isTruthyEnv(env[NO_SKILL_REGISTRY_ENV]) || hasCliArg(argv, "--no-skills", "-ns") diff --git a/extensions/startup-banner.ts b/extensions/startup-banner.ts index 57c31c1d0..dffcceecc 100644 --- a/extensions/startup-banner.ts +++ b/extensions/startup-banner.ts @@ -647,6 +647,9 @@ export default function (pi: ExtensionAPI) { pi.on("session_start", async (_event, ctx) => { disposeHeader(); if (!ctx.hasUI) return; + // Delegated rpc children report hasUI=true but have no terminal to paint + // (gentle-shell#1690); do not rely on a piped stdout lacking rows/columns. + if (process.env.GENTLE_PI_AGENTS_CHILD === "1") return; // CLI subcommands such as `pi update` or `pi install` skip the animated intro. if (isPiCliSubcommandInvocation(process.argv)) return; diff --git a/odd/tasks/fix-1690-standalone-child-package.md b/odd/tasks/fix-1690-standalone-child-package.md new file mode 100644 index 000000000..50e922438 --- /dev/null +++ b/odd/tasks/fix-1690-standalone-child-package.md @@ -0,0 +1,119 @@ +# Fix #1690: standalone subagents load the gentle-pi package + +## Objective + +In isolated standalone Gentle Shell, delegated children (`pi --mode rpc`) must load the same gentle-pi package as the parent. Acceptance target: an isolated child behaves like a regular gentle-pi child, where `settings.json` declares the package. + +## Problem + +In isolated mode the launcher injects the package into the parent only through `pi -e ` (`lib/gentle-shell-launcher.ts` `buildPiInvocation`, ~:918-947), and setup removes `npm:gentle-pi` from the isolated home `settings.json`. The runner never forwards that injection. `childArguments` (`lib/agents-runner.ts:258`) adds `--extension` only for `request.extensionPaths`, which holds just `child-context.ts` and `child-safety.ts` (`extensions/gentle-agents.ts:114`, `:148`, `:1304`). Children lose the guardrails gate, `subagent_parent_message`, session-change capture (#1688), codegraph, `gentle_review_scope`, the review permission relay, and package skills/prompts. Pi drops unknown `--tools` names silently. + +## Decided direction + +barbatdev on #1690 (2026-10-03 04:19Z): + +1. Forward the parent's package to children when the parent received it through `-e`, not through `settings.json`. +2. Move the orchestrator-only filtering that `child-context.ts` does today into the fully loaded package. The curated child-entrypoint list goes away. +3. The runner warns when a tool requested in `--tools` does not exist in the child. + +## Design constraints (verified on origin/main ac671593, Pi 1.0.0) + +- No grandchildren: `agentsEnabled()` returns false when `GENTLE_PI_AGENTS_CHILD=1` (`extensions/gentle-agents.ts:152`). +- Prerequisite: `gentle-ai.ts` `session_start` (~:9673) has no child guard. It runs `installPackageAssets`, `migrateLegacyProjectModelOverrides` and `applySavedModelConfig` (~:9715-9717). Forwarding the package without a guard would make every child rewrite shared files. Regular gentle-pi children probably hit this today already. +- The launcher must signal the injection explicitly (env), not leave the child to infer it from argv or `import.meta.url`. +- Launcher cases: no declaration → forward `packageRoot`. Takeover (`--no-extensions` + other `-e` set) → forward the full set or define a scope. A declaration already in settings → forward nothing, or the package registers twice. +- Once the package is forwarded, drop `child-context.ts`/`child-safety.ts` from the child args, or make them idempotent. +- Pi 1.0 RPC has no tool listing (no `get_tools`). The missing-tool warning needs either a child-side self-check reported over the child IPC, or a parent-side check. Verify before choosing. +- Windows: pass paths as plain argv elements, absolute, with no shell quoting. +- Overlaps: #593 / PR #605 (child extension selection), #1688 (child capture; GuidoCarda has a local fix). + +## Constraints + +- Technical artifacts in English. Test-first wherever there is a deterministic runnable test. +- No PR until barbatdev answers whether to wait for `status:approved` (the issue has `status:needs-review,type:bug`). +- Out of scope: #1647, #1064/#1557/#1558. + +## Tasks + +- [x] T1 Hook audit: classify every gentle-pi package hook that would newly run in a child (session_start, before_agent_start, tool_call, timers, UI, registrations) as must-run, must-skip or harmless. Record the table in this document. Route: delegated read-only (`gentle-ai-explore`, task mus95p0g-1-r2m0); the parent spot-checked `hasUI` in the Pi rpc loader. See "Child hook audit". Commit `5848fb80`. +- [x] T2 Child guards for the must-skip items in "Child hook audit" (gentle-ai session_start writes, skill-registry, history, pi-pretty fallback, optional startup-banner), with tests. Route: delegated writer (musez9uu-2-2u6i), verified by `gentle-ai-verify` (musfh6tb-3-entu). The child also skips the review-permission revoke/refresh: the grant is host-only (`lib/review-session-standing-permission.ts:162`) and the child relay is created at load. Commit: see Evidence. +- [x] T2b Review follow-ups from the T2 RDD review (non-blocking): (a) WARNING `tests/gentle-ai-child-guards.test.ts:83-93`: the "keeps local resets" test does not assert that the child-local resets still run; add a real assertion. (b) SUGGESTION `extensions/gentle-ai.ts:9681-9686`: the early return skips any session state initialized further down; wrap only the parent steps in the guard, or assert the child state explicitly. Route: delegated writer (musgdawc-4-k2ug). Done: (a) the tests now prove that the child elapsed-timing ledger and the reminder re-arm run (RED 2/6 with the guard moved above the resets; GREEN 6/6); (b) the parent-only work moved into `startParentSession`, which children never call. `tests/gentle-ai.test.ts` 102/102; typecheck shows no regressions. Known gaps: `yolo.reset`/`reviewSidebar.reset` have no direct assertion, and the `reminderManager` reset is proven only in the green direction. Commit: the T2b commit carrying this line. +- [ ] T3 Launcher injection signal: `buildPiInvocation` exports the injected extension set (and the takeover flag) to the parent env for the three cases, with tests. Route: per the ladder. +- [ ] T4 Runner forwarding: gentle-agents builds the child extension args from that signal (takeover set with `--no-extensions`; nothing when declared) and drops the curated entries when the package is forwarded, with tests. Route: per the ladder. +- [ ] T5 Move the `child-context.ts`/`child-safety.ts` behavior into the loaded package, gated on `GENTLE_PI_AGENTS_CHILD`, with no double registration, with tests. Route: per the ladder. +- [ ] T6 Missing `--tools` warning: choose between a child self-check over IPC and a parent-side check (verify first), implement, test. Route: per the ladder. +- [ ] T7 Acceptance probe: spawn like the runner (`--mode rpc --session-dir `, `GENTLE_PI_AGENTS_CHILD=1`, isolated home) and compare RPC `get_commands` with a regular gentle-pi child (baseline 18 `/gentle:*` vs 0); measure startup cost; update docs. Route: `gentle-ai-verify`. + +## Child hook audit (T1) + +Audited statically on ac671593 against Pi 1.0.0 `@earendil-works/pi-coding-agent/dist`. + +**Loader facts** + +- `pi.extensions: ["./extensions"]` has no `index.ts`. Pi loads every top-level `extensions/*.ts` file plus `extensions/history/index.ts`, 18 entrypoints in total (`package-manager.js:377-456`). `child-context.ts` and `child-safety.ts` are therefore already package entrypoints. +- Pi dedupes extension paths by canonical path, CLI paths first (`package-manager.js:2058-2062`, `resource-loader.js:316-318,656-668`). Forwarding `-e ` together with `--extension child-*.ts` loads each file once. +- In an rpc child, `ctx.hasUI` is **true**: `rpc-mode.js:231` binds an RPC UI context and `runner.js:363` defines `hasUI`. Every `hasUI` guard takes the UI branch in children. Select/confirm/input/editor calls become a task ASK to the parent (`agents-protocol.ts:154,332-333`). notify/setStatus/setWidget calls are dropped. + +**Must-skip in children (new `GENTLE_PI_AGENTS_CHILD` guard)** + +1. `gentle-ai.ts:9715-9717` session_start: `installPackageAssets(force)`, `migrateLegacyProjectModelOverrides`, `applySavedModelConfig`. All three write shared state. +2. `skill-registry.ts:631` session_start: writes `.atl/` into the child cwd, renames the legacy registry, and starts a recursive fs watcher (gated on `hasUI`, which is true). +3. `history/index.ts:1361-1399`: writer init, `before_agent_start` prompt capture (would record delegation prompts as user history), GC. +4. `pi-pretty.ts:42`: the `!shellEnabled` fallback loads upstream pi-pretty with FFF indexing per child. +5. Optional hardening: `startup-banner.ts:647` is skipped today only because a piped stdout has no rows/cols. + +**Uncertain, decide in T2** + +- `gentle-ai.ts:9683-9696,9744-9747` session_start repository-preparation binding and review-status negotiation (spawns the native CLI per child); `tool_result` `recordReviewMutation`/`prepareBoundSessionRepository` (`:9854-9866`). +- Guardrails `confirmCommand` (`gentle-ai.ts:1822-1828`): its headless block is skipped because `hasUI` is true, so commands classed "confirm" in a child would ASK the parent user and the task would wait. Today they run unprompted. **Decided by the user (2026-10-03): the child asks the parent for confirmation.** Keep the ASK path, no child bypass; cover it with a test. +- Parent decision for T2: the repository-preparation binding, startup review negotiation and `prepareBoundSessionRepository` belong to the parent session (the child already runs in the parent's resolved worktree), so skip them in children. Keep the child-local resets and `recordReviewMutation` (child session only). The writer verifies whether review-permission revoke/refresh must stay for the child relay. + +**Must-run (gained by forwarding)** + +The gentle-ai `tool_call` guardrails, review relay handshake and review tools; gentle-shell session-change capture (runs before the shell guard); gentle-agents child `subagent_parent_message`; codegraph; nan-provider; child-context and child-safety. + +**Already harmless in children** + +gentle-shell UI/timers, the gentle-agents host, gentle-todo, runtime-metrics, gentle-stats, resume-hint, ask-user-*, quiet-tools, and the gentle-ai `before_agent_start`/`agent_end` (child-guarded). + +## Working layout + +- T1-T2 live on `fix/1690-standalone-child-package` (worktree `gentle-shell-worktrees/fix-1690-standalone-child-package`). Its RDD review of `a67bb7f5` runs from a separate native Claude Code session, and nothing else writes in that worktree while the review is open. +- T3 onward continue on `fix/1690-child-package-forwarding` (worktree `gentle-shell-worktrees/fix-1690-child-package-forwarding`), stacked on `a67bb7f5`. If the review adds a correction commit, rebase this branch onto it before delivery. + +## Delivery budget + +As of `f384d2a1`, the branch carries 322 changed lines against origin/main in code and tests (T2), plus about 100 in this ODD document, roughly 420 in total. The forecast for T2b-T7 is about 600-800 more lines, so a single PR would exceed the 400-line budget several times over. Proposed: stacked PRs to main, each landable on its own: + +1. PR1, child guards: T1 + T2 + T2b. On its own it changes nothing for isolated children, and it hardens regular gentle-pi children, which already load the package. About 460 lines including this document; slightly over budget because of the tests and the doc. +2. PR2, package forwarding: T3 + T4 + T5. +3. PR3, the missing-tool warning, the acceptance probe and docs: T6 + T7. + +**Chain strategy: stacked PRs to main, confirmed by the user on 2026-10-03.** Branch plan: + +- PR1: `fix/1690-standalone-child-package`. Once T2b is committed here, fast-forward that branch to include it (the docs commit plus T2b). +- PR2: a new branch from the PR1 tip, for T3-T5. +- PR3: a new branch from the PR2 tip, for T6-T7. + +Every PR carries the chain context and a dependency diagram (chained-pr skill). No PR before barbatdev answers on `status:approved`. + +## Pending follow-ups + +- [ ] **Report upstream: `gentle_review` unreachable from Pi over pi-claude-bridge.** Target: pi-claude-bridge (elidickinson) or gentle-shell; decide after confirming the cause. + - Symptom (2026-10-03, Gentle Shell standalone, Pi 1.0.0, pi-claude-bridge 0.9.0, model Opus 5.5): the gentle-pi tool `gentle_review` (registered unconditionally, `extensions/gentle-ai.ts:9526`) never reaches the model. `gentle_review_capture`, `gentle_review_capture_group` and `gentle_review_scope` do. The model's own instructions say some tools are deferred, and a SessionStart hook asks it to run `ToolSearch`, which it does not have. + - Hypothesis (not verified): the bridge provider path starts Claude Code with `tools: []` (`pi-claude-bridge/src/index.ts:1960`, comments at :136 and :918), so the built-in `ToolSearch` is unavailable, while Claude Code still defers part of the MCP tool set. Deferred tools then become unreachable. `ToolSearch` is also always blocked in AskClaude mode (`src/index.ts:168`). + - Impact: RDD inspect/START cannot run from a Pi session on this provider. Workaround in use: run the reviews from a native Claude Code session. + - To confirm before filing: restart with `ENABLE_TOOL_SEARCH=false` (all tools sent upfront) and check that `gentle_review` appears; check whether the deferral depends on tool count or schema size; search existing bridge issues (related: #153, Pi 1.0 mcp_servers). + +## Acceptance criteria + +- An isolated standalone child exposes the same package commands and tools as a regular gentle-pi child. +- Child launches do not rewrite shared files (assets, agent frontmatter, `subagents.json`, project `.pi/settings.json`). +- The package registers exactly once in every launcher case (no declaration, takeover, declaration). +- A requested tool that is missing in the child produces a visible warning. + +## Evidence + +- T1: commit `5848fb80` (static audit; the loader facts were re-confirmed on Pi 1.0.0 during T2 verification). +- T2 RDD review (native Claude Code session): `approved`, medium risk, one lens (review-reliability), candidate `a67bb7f5` against base `5848fb80`, no correction; authority burned (lineage `review-71c63d42e08bde90`). Two non-blocking findings were tracked as T2b. +- T2: RED→GREEN per guard (writer). Verification on a real `pnpm install --frozen-lockfile` with pi-coding-agent 1.0.0: focused tests 55/55; wider set 517/518, the one failure being `tests/history-session-scan-extract.test.ts:197` (mtime vs `Date.now()`, untouched code), which then passed 14/14 three times in isolation, so treated as a timing flake; `node scripts/check-types.mjs` gives the same result on the branch and on origin/main (186 recorded, no regressions). Loader facts on 1.0.0: rpc `hasUI` is true (`rpc-mode.js:230-232`, `runner.js:404-405`); `mergePaths` dedupes by realpath with the CLI paths first (`resource-loader.js:403-405,781-792`). Commit: the T2 commit carrying this line. diff --git a/tests/gentle-ai-child-guards.test.ts b/tests/gentle-ai-child-guards.test.ts new file mode 100644 index 000000000..b83b1e75b --- /dev/null +++ b/tests/gentle-ai-child-guards.test.ts @@ -0,0 +1,202 @@ +import assert from "node:assert/strict"; +import { mkdirSync, mkdtempSync, readdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test, { type TestContext } from "node:test"; +import type { ExtensionAPI, ExtensionContext, ToolDefinition } from "@earendil-works/pi-coding-agent"; +import { createGentleAiExtension } from "../extensions/gentle-ai.ts"; +import { bindSessionRepositoryPreparation } from "../lib/bounded-writer-admission.ts"; +import { GENTLE_AI_TIMING_ENTRY } from "../lib/gentle-ai-elapsed-store.ts"; +import { REVIEW_SIDEBAR_EVENT } from "../lib/review-sidebar-state.ts"; +import { YOLO_STATUS_KEY } from "../lib/yolo-session-policy.ts"; + +// gentle-shell#1690: the package is forwarded to delegated rpc children +// (GENTLE_PI_AGENTS_CHILD=1, where ctx.hasUI is true). Parent-owned startup +// work and shared-state writes must not run there; each case pairs the child +// with a parent control so the guard cannot pass vacuously. + +type Handler = (event: unknown, ctx: ExtensionContext) => unknown; + +function isolate(t: TestContext): { root: string; cwd: string; agentHome: string } { + const root = realpathSync(mkdtempSync(join(tmpdir(), "gentle-pi-child-guards-"))); + const cwd = join(root, "project"); + const agentHome = join(root, "agent-home"); + const configHome = join(root, "config"); + for (const path of [cwd, agentHome, configHome]) mkdirSync(path, { recursive: true }); + const previous = { agentHome: process.env.GENTLE_PI_AGENT_HOME, configHome: process.env.GENTLE_PI_CONFIG_HOME }; + process.env.GENTLE_PI_AGENT_HOME = agentHome; + process.env.GENTLE_PI_CONFIG_HOME = configHome; + t.after(() => { + if (previous.agentHome === undefined) delete process.env.GENTLE_PI_AGENT_HOME; + else process.env.GENTLE_PI_AGENT_HOME = previous.agentHome; + if (previous.configHome === undefined) delete process.env.GENTLE_PI_CONFIG_HOME; + else process.env.GENTLE_PI_CONFIG_HOME = previous.configHome; + rmSync(root, { recursive: true, force: true }); + }); + return { root, cwd, agentHome }; +} + +function harness(child: boolean, cwd: string) { + const handlers = new Map(); + const entries: Array<{ type: string; customType: string; data: unknown }> = []; + const nativeAccesses: string[] = []; + const sweeps: string[] = []; + const prompts: string[] = []; + const statuses: Array<[string, string | undefined]> = []; + const emitted: Array<{ name: string; payload: unknown }> = []; + const tools = new Map(); + const pi = { + on: (name: string, handler: Handler) => { handlers.set(name, handler); }, + events: { on() {}, emit: (name: string, payload: unknown) => { emitted.push({ name, payload }); } }, + appendEntry: (customType: string, data: unknown) => { entries.push({ type: "custom", customType, data }); }, + getThinkingLevel: () => "medium", + registerTool: (tool: ToolDefinition) => { tools.set(tool.name, tool); }, + registerCommand() {}, registerShortcut() {}, registerMessageRenderer() {}, + } as unknown as ExtensionAPI; + // Any read of the native CLI means the session reached review negotiation + // or repository preparation. + const nativeReviewCli = new Proxy({}, { + get: (_target, key) => { nativeAccesses.push(String(key)); return undefined; }, + }); + const candidateViews = { sweepOrphans: (path: string) => { sweeps.push(path); }, cleanupAll() {} }; + createGentleAiExtension({ + processEnv: { GENTLE_PI_AGENTS_CHILD: child ? "1" : "0", GENTLE_AI_TELEMETRY: "0" }, + nativeReviewCli: nativeReviewCli as never, + candidateViews: candidateViews as never, + childStandingReviewPermissionClient: { requestAuthorization: async () => false, close() {} }, + })(pi); + const sessionManager = { getSessionId: () => `session-${child ? "child" : "parent"}`, getCwd: () => cwd, getEntries: () => entries, getBranch: () => entries }; + const ctx = { + cwd, + mode: "rpc", + hasUI: true, + sessionManager, + ui: { + notify() {}, + setStatus: (key: string, text?: string) => { statuses.push([key, text]); }, + confirm: async (title: string) => { prompts.push(`confirm:${title}`); return false; }, + select: async (title: string) => { prompts.push(`select:${title}`); return undefined; }, + }, + } as unknown as ExtensionContext; + return { handlers, entries, nativeAccesses, sweeps, prompts, statuses, emitted, tools, ctx, sessionManager }; +} + +function legacySettings(cwd: string): string { + const path = join(cwd, ".pi", "settings.json"); + mkdirSync(join(cwd, ".pi"), { recursive: true }); + writeFileSync(path, `${JSON.stringify({ subagents: { agentOverrides: { worker: "openai/gpt-5" } } }, null, 2)}\n`); + return path; +} + +test("child session_start keeps local resets but skips parent-owned startup work", async (t) => { + const { cwd, agentHome } = isolate(t); + const settingsPath = legacySettings(cwd); + const before = readFileSync(settingsPath, "utf8"); + const h = harness(true, cwd); + const timings = () => h.entries.filter((entry) => entry.customType === GENTLE_AI_TIMING_ENTRY); + await h.handlers.get("tool_execution_start")!({ toolName: "gentle_review", toolCallId: "before" }, h.ctx); + assert.equal(timings().length, 0, "no elapsed-timing ledger exists before session_start"); + await h.handlers.get("session_start")!({ reason: "startup" }, h.ctx); + await h.handlers.get("tool_execution_start")!({ toolName: "gentle_review", toolCallId: "after" }, h.ctx); + assert.equal(timings().length, 1, "the child still creates its elapsed-timing ledger"); + assert.deepEqual(readdirSync(agentHome), [], "a child must not install package assets"); + assert.equal(readFileSync(settingsPath, "utf8"), before, "a child must not migrate project model overrides"); + assert.deepEqual(h.sweeps, [], "a child must not sweep candidate views"); + assert.deepEqual(h.nativeAccesses, [], "a child must not negotiate review status or bind repository preparation"); +}); + +test("parent session_start still runs the startup work skipped in children", async (t) => { + const { cwd, agentHome } = isolate(t); + const settingsPath = legacySettings(cwd); + const before = readFileSync(settingsPath, "utf8"); + const h = harness(false, cwd); + await h.handlers.get("session_start")!({ reason: "startup" }, h.ctx); + assert.notDeepEqual(readdirSync(agentHome), [], "the parent installs package assets"); + assert.notEqual(readFileSync(settingsPath, "utf8"), before, "the parent migrates legacy project model overrides"); + assert.deepEqual(h.sweeps, [cwd]); + assert.ok(h.nativeAccesses.length > 0, "the parent negotiates review status"); +}); + +test("child session_start re-arms the reminder session for its own session manager", async (t) => { + const { cwd } = isolate(t); + mkdirSync(join(cwd, "src"), { recursive: true }); + writeFileSync(join(cwd, "src", "a.ts"), "export {};\n"); + const h = harness(true, cwd); + const next = { ...h.sessionManager, getSessionId: () => "session-child-next" }; + const nextCtx = { ...h.ctx, sessionManager: next } as unknown as ExtensionContext; + const write = (ctx: ExtensionContext, toolCallId: string) => h.handlers.get("tool_result")!({ toolName: "write", toolCallId, isError: false, input: { path: "src/a.ts" } }, ctx); + const mutations = () => h.entries.filter((entry) => (entry.data as { kind?: string })?.kind === "mutation"); + await h.handlers.get("session_start")!({ reason: "startup" }, h.ctx); + await h.handlers.get("session_shutdown")!({ reason: "new" }, h.ctx); + await write(h.ctx, "after-shutdown"); + assert.equal(mutations().length, 0, "a shut-down session records nothing"); + await h.handlers.get("session_start")!({ reason: "new" }, nextCtx); + await write(h.ctx, "stale-manager"); + assert.equal(mutations().length, 0, "the reminder manager moved to the new session"); + await write(nextCtx, "current-manager"); + assert.equal(mutations().length, 1, "the restarted child records its own mutation"); +}); + +test("child session_start re-arms YOLO and the review sidebar for its own session", async (t) => { + const { cwd } = isolate(t); + const h = harness(true, cwd); + const next = { ...h.sessionManager, getSessionId: () => "session-child-next" }; + const nextCtx = { ...h.ctx, sessionManager: next } as unknown as ExtensionContext; + const yoloClears = () => h.statuses.filter(([key, text]) => key === YOLO_STATUS_KEY && text === undefined).length; + const sidebarSessions = () => h.emitted.filter((event) => event.name === REVIEW_SIDEBAR_EVENT) + .map((event) => (event.payload as { sessionId: string }).sessionId); + // Any non-assess call passes through the sidebar wrapper, which publishes + // before running; the native outcome itself is irrelevant here. + const capture = async (ctx: ExtensionContext, toolCallId: string) => { + try { await h.tools.get("gentle_review_capture")!.execute(toolCallId, {}, undefined, undefined, ctx as never); } catch { /* outcome irrelevant */ } + }; + await h.handlers.get("session_start")!({ reason: "startup" }, h.ctx); + await h.handlers.get("session_shutdown")!({ reason: "new" }, h.ctx); + await capture(h.ctx, "after-shutdown"); + assert.deepEqual(sidebarSessions(), [], "a shut-down sidebar publishes nothing"); + const clearsBeforeRestart = yoloClears(); + await h.handlers.get("session_start")!({ reason: "new" }, nextCtx); + assert.equal(yoloClears(), clearsBeforeRestart + 1, "the child session_start resets YOLO and clears its indicator"); + await capture(h.ctx, "stale-session"); + assert.deepEqual(sidebarSessions(), [], "the sidebar moved to the new session"); + await capture(nextCtx, "current-session"); + assert.ok(sidebarSessions().length > 0, "the restarted child sidebar publishes again"); + assert.ok(sidebarSessions().every((id) => id === "session-child-next"), JSON.stringify(sidebarSessions())); +}); + +async function writeToolResult(child: boolean, t: TestContext) { + const { cwd } = isolate(t); + mkdirSync(join(cwd, "src"), { recursive: true }); + writeFileSync(join(cwd, "src", "a.ts"), "export {};\n"); + const h = harness(child, cwd); + await h.handlers.get("session_start")!({ reason: "startup" }, h.ctx); + // Bind an observable preparation on the same session owner the parent + // session_start would have bound, so only the tool_result guard decides. + const preparations: string[] = []; + const unbind = bindSessionRepositoryPreparation(h.sessionManager, cwd, async (root) => { preparations.push(root); return false; }, () => true); + t.after(unbind); + await h.handlers.get("tool_result")!({ toolName: "write", toolCallId: "call-1", isError: false, input: { path: "src/a.ts" } }, h.ctx); + const mutations = h.entries.filter((entry) => (entry.data as { kind?: string })?.kind === "mutation"); + return { cwd, preparations, mutations }; +} + +test("child tool_result records the mutation but never prepares the bound repository", async (t) => { + const { preparations, mutations } = await writeToolResult(true, t); + assert.equal(mutations.length, 1, "the child keeps its own mutation receipt"); + assert.deepEqual(preparations, []); +}); + +test("parent tool_result still prepares the bound repository", async (t) => { + const { cwd, preparations, mutations } = await writeToolResult(false, t); + assert.equal(mutations.length, 1); + assert.deepEqual(preparations, [cwd]); +}); + +test("child confirm-class guardrail commands ask the parent instead of running or blocking headlessly", async (t) => { + const { cwd } = isolate(t); + const h = harness(true, cwd); + const result = await h.handlers.get("tool_call")!({ toolName: "bash", input: { command: "pi remove some-package" } }, h.ctx) as { block?: boolean; reason?: string } | undefined; + assert.equal(h.prompts.length, 1, JSON.stringify(h.prompts)); + assert.equal(result?.block, true, "the declined confirmation blocks the command"); + assert.doesNotMatch(result?.reason ?? "", /requires interactive confirmation/); +}); diff --git a/tests/history-child-guard.test.ts b/tests/history-child-guard.test.ts new file mode 100644 index 000000000..ffda7260c --- /dev/null +++ b/tests/history-child-guard.test.ts @@ -0,0 +1,70 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import promptHistoryExtension from "../extensions/history/index.ts"; + +// gentle-shell#1690: delegated children (GENTLE_PI_AGENTS_CHILD=1) receive +// the package too. Their before_agent_start prompt is a delegation brief, +// not user history, so a child must never initialize, capture or GC the +// store, even when capture is opted in. The selector may still register. + +const CWD = "/pi-history-test/project-child"; + +type Handler = (...args: unknown[]) => unknown; + +function load(child: boolean) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "pi-history-child-")); + const handlers = new Map(); + const commands: string[] = []; + const shortcuts: string[] = []; + const pi = { + on: (name: string, handler: Handler) => { + handlers.set(name, [...(handlers.get(name) ?? []), handler]); + }, + registerShortcut: (key: string) => { shortcuts.push(key); }, + registerCommand: (name: string) => { commands.push(name); }, + }; + promptHistoryExtension(pi as never, { + env: { GENTLE_PI_HISTORY_CAPTURE: "1", GENTLE_PI_AGENTS_CHILD: child ? "1" : "0" }, + root, + cwd: CWD, + instanceId: `inst-${child ? "child" : "parent"}`, + now: () => 1700000000000, + agentDir: path.join(root, "agent"), + sessionsRoot: path.join(root, "sessions"), + gentlePiConfigHome: fs.mkdtempSync(path.join(os.tmpdir(), "pi-history-child-config-")), + }); + const fire = async (name: string, event: unknown) => { + for (const handler of handlers.get(name) ?? []) await handler(event, {}); + }; + return { root, fire, commands, shortcuts }; +} + +const flush = () => new Promise((resolve) => setImmediate(resolve)); + +test("a delegated child never initializes, captures or GCs prompt history", async () => { + const { root, fire, commands, shortcuts } = load(true); + await flush(); + assert.deepEqual(fs.readdirSync(root), [], "no warm-up writer init"); + await fire("before_agent_start", { prompt: "delegated task brief" }); + assert.deepEqual(fs.readdirSync(root), [], "no prompt capture"); + await fire("session_shutdown", {}); + assert.deepEqual(fs.readdirSync(root), [], "no GC rewrite"); + assert.deepEqual(commands, ["history"]); + assert.equal(shortcuts.length, 1); +}); + +test("an opted-in parent still captures prompt history", async () => { + const { root, fire } = load(false); + await flush(); + await fire("before_agent_start", { prompt: "parent prompt" }); + const files = fs.readdirSync(root, { recursive: true }).map(String); + assert.ok(files.length > 0, "the parent initializes its store"); + const captured = files + .map((file) => path.join(root, file)) + .filter((file) => fs.statSync(file).isFile()) + .some((file) => fs.readFileSync(file, "utf8").includes("parent prompt")); + assert.ok(captured, `the parent prompt is persisted: ${JSON.stringify(files)}`); +}); diff --git a/tests/pi-pretty.test.ts b/tests/pi-pretty.test.ts index aab3c5057..d22cdbfd0 100644 --- a/tests/pi-pretty.test.ts +++ b/tests/pi-pretty.test.ts @@ -48,3 +48,13 @@ test("disabled shell leaves bundled editor behavior untouched", async () => { await pretty(pi, undefined, async (api: unknown) => { received = api; }, { GENTLE_PI_SHELL: "0" }); assert.equal(received, pi); }); + +// gentle-shell#1690: a delegated child has no shell (shellEnabled is false), +// so the upstream fallback would start FFF indexing in every child. +test("delegated children never load upstream pi-pretty", async () => { + let loaded = false; + const pi = {}; + const result = await pretty(pi, undefined, async () => { loaded = true; }, { GENTLE_PI_AGENTS_CHILD: "1" }); + assert.equal(loaded, false); + assert.equal(result, undefined); +}); diff --git a/tests/skill-registry.test.ts b/tests/skill-registry.test.ts index 8812093f7..547a9f83b 100644 --- a/tests/skill-registry.test.ts +++ b/tests/skill-registry.test.ts @@ -8,6 +8,10 @@ import { pathToFileURL } from "node:url"; import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; import skillRegistry, { __testing } from "../extensions/skill-registry.ts"; +// Registered startup reads process.env directly; a suite launched from a +// delegated child must still exercise the parent paths (gentle-shell#1690). +delete process.env.GENTLE_PI_AGENTS_CHILD; + test("project skill dirs include supported workspace roots", () => { const cwd = "/repo"; const dirs = __testing.projectSkillDirs(cwd); @@ -178,6 +182,8 @@ test("startup skip honors no skill registry controls", () => { true, ); assert.equal(__testing.shouldSkipSkillRegistryStartup(disabled, [], {}), false); + assert.equal(__testing.shouldSkipSkillRegistryStartup(disabled, [], { GENTLE_PI_AGENTS_CHILD: "1" }), true); + assert.equal(__testing.shouldSkipSkillRegistryStartup(disabled, [], { GENTLE_PI_AGENTS_CHILD: "0" }), false); }); test("duplicate extension load is skipped only across different sources", () => { @@ -551,6 +557,31 @@ for (const control of ["environment", "--no-skills", "-ns"]) { }); } +// gentle-shell#1690: delegated rpc children have hasUI=true but must not write +// .atl/, rename the legacy registry or start a watcher in the shared cwd. +test("registered startup in a delegated child avoids writes, legacy rename and watchers", async (t) => { + const fixture = registryFixture(); + const legacy = join(fixture.cwd, ".pi", "extensions", "skill-registry.ts"); + mkdirSync(dirname(legacy), { recursive: true }); + const source = 'Auto-generated by .pi/extensions/skill-registry.ts\nconst REGISTRY_REL_PATH = ".atl/skill-registry.md"\nfunction projectSkillDirs(cwd: string): string[]\nfunction regenerateRegistry(cwd: string, force: boolean)\n'; + writeFileSync(legacy, source); + const previous = process.env.GENTLE_PI_AGENTS_CHILD; + process.env.GENTLE_PI_AGENTS_CHILD = "1"; + t.after(() => { + if (previous === undefined) delete process.env.GENTLE_PI_AGENTS_CHILD; + else process.env.GENTLE_PI_AGENTS_CHILD = previous; + }); + const runtime = registeredRegistry(fixture.cwd); + t.after(() => runtime.stop()); + await runtime.start(); + assert.equal(existsSync(fixture.registry), false); + assert.equal(existsSync(fixture.ignore), false); + assert.equal(readFileSync(legacy, "utf8"), source); + assert.equal(existsSync(`${legacy}.disabled`), false); + assert.equal(__testing.activeWatcherCount(), 0); + assert.deepEqual(runtime.notices, []); +}); + test("registered suppression avoids writes and watchers but permits explicit refresh", async (t) => { const fixture = registryFixture(); const runtime = registeredRegistry(fixture.cwd, true); diff --git a/tests/startup-banner.test.ts b/tests/startup-banner.test.ts index 211334149..3573e3388 100644 --- a/tests/startup-banner.test.ts +++ b/tests/startup-banner.test.ts @@ -10,6 +10,10 @@ import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; import { visibleWidth } from "@earendil-works/pi-tui"; import { stripAnsi } from "../lib/terminal-theme.ts"; +// The banner reads process.env directly; a suite launched from a delegated +// child must still exercise the parent paths (gentle-shell#1690). +delete process.env.GENTLE_PI_AGENTS_CHILD; + test("startup artwork spells Gentle Shell with aligned animation spans", () => { const source = readFileSync(new URL("../extensions/startup-banner.ts", import.meta.url), "utf8"); const logo = JSON.parse(source.match(/const TEXT_LOGO = (\[[\s\S]*?\]);/)![1].replace(/,\s*]/, "]")) as string[]; @@ -396,3 +400,45 @@ test("launcher-injected extension directories do not suppress the startup banner assert.equal(isPiCliSubcommandInvocation(["node", "pi", sub, "npm:x"]), true, sub); } }); + +// gentle-shell#1690: a delegated rpc child has hasUI=true; only a piped stdout +// without rows/columns kept the banner off before the explicit child guard. +test("delegated children never paint the startup banner", async (t) => { + const previousChild = process.env.GENTLE_PI_AGENTS_CHILD; + process.env.GENTLE_PI_AGENTS_CHILD = "1"; + t.after(() => { + if (previousChild === undefined) delete process.env.GENTLE_PI_AGENTS_CHILD; + else process.env.GENTLE_PI_AGENTS_CHILD = previousChild; + }); + const home = mkdtempSync(join(tmpdir(), "gp-banner-child-")); + const previousHome = process.env.GENTLE_PI_CONFIG_HOME; + process.env.GENTLE_PI_CONFIG_HOME = home; + t.after(() => { + if (previousHome === undefined) delete process.env.GENTLE_PI_CONFIG_HOME; + else process.env.GENTLE_PI_CONFIG_HOME = previousHome; + rmSync(home, { recursive: true, force: true }); + }); + t.mock.timers.enable({ apis: ["setTimeout", "setInterval", "Date"] }); + t.mock.method(fs, "readFile", async () => JSON.stringify({ showRose: true, showTextLogo: true, color: "pink" })); + syncBuiltinESMExports(); + t.after(() => { t.mock.restoreAll(); syncBuiltinESMExports(); }); + const argv = process.argv; + process.argv = ["node"]; + t.after(() => { process.argv = argv; }); + for (const [key, value] of [["rows", 40], ["columns", 160]] as const) { + const descriptor = Object.getOwnPropertyDescriptor(process.stdout, key); + Object.defineProperty(process.stdout, key, { configurable: true, writable: true, value }); + t.after(() => descriptor ? Object.defineProperty(process.stdout, key, descriptor) : Reflect.deleteProperty(process.stdout, key)); + } + let start: Function; + let shutdown: Function; + startup({ on: (name: string, fn: Function) => { + if (name === "session_start") start = fn; + if (name === "session_shutdown") shutdown = fn; + }, registerCommand() {}, getCommands: () => [], getAllTools: () => [] } as unknown as ExtensionAPI); + t.after(() => shutdown?.()); + let headers = 0; + await start!({}, { hasUI: true, cwd: "/fixture", ui: { setHeader: () => { headers++; } } }); + t.mock.timers.tick(50); + assert.equal(headers, 0); +});