diff --git a/bin/gentle-shell.mjs b/bin/gentle-shell.mjs index 8550767d4..e80180cd6 100755 --- a/bin/gentle-shell.mjs +++ b/bin/gentle-shell.mjs @@ -1383,6 +1383,8 @@ async function main() { piSubcommand: args.piSubcommand, baseEnv: process.env, homedir: homedir(), + // The spawn below sets no cwd, so pi runs in the launcher's own. + cwd: process.cwd(), }); // Only an interactive session ends with pi's exit resume hint, which 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/lib/child-package-injection.ts b/lib/child-package-injection.ts new file mode 100644 index 000000000..ce887deec --- /dev/null +++ b/lib/child-package-injection.ts @@ -0,0 +1,62 @@ +import { isAbsolute } from "node:path"; + +// #1690: the gentle-shell launcher injects the gentle-pi package into the +// parent pi with `-e` instead of a settings declaration, so delegated children +// would otherwise start without it. The launcher records the exact extension +// set it injected in this variable; the subagent runner forwards that set to +// every child. Kept free of launcher imports so the runner can load it cheaply. +export const CHILD_PACKAGE_INJECTION_ENV = "GENTLE_SHELL_CHILD_PACKAGE_INJECTION"; + +const CHILD_PACKAGE_INJECTION_VERSION = 1; + +export interface ChildPackageInjection { + // True for a launcher takeover: the parent ran with --no-extensions, so the + // child must too, or settings discovery would load a second gentle-pi. + noExtensions: boolean; + // Absolute extension paths, in the order the launcher passed them to `-e`. + extensionPaths: string[]; +} + +export function encodeChildPackageInjection(value: ChildPackageInjection): string { + return JSON.stringify({ + version: CHILD_PACKAGE_INJECTION_VERSION, + noExtensions: value.noExtensions, + extensionPaths: value.extensionPaths, + }); +} + +// Returns undefined for an absent or invalid value and never throws: a bad +// signal must degrade to "no forwarding", not break a child launch. The path +// flavor is injectable so tests can check Windows paths on any platform. +export function parseChildPackageInjection( + env: Record, + pathFlavor: { isAbsolute(path: string): boolean } = { isAbsolute }, +): ChildPackageInjection | undefined { + const raw = env[CHILD_PACKAGE_INJECTION_ENV]; + if (raw === undefined || raw.length === 0) return undefined; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return undefined; + } + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return undefined; + const record = parsed as Record; + if (record.version !== CHILD_PACKAGE_INJECTION_VERSION) return undefined; + if (typeof record.noExtensions !== "boolean") return undefined; + const paths = record.extensionPaths; + if (!Array.isArray(paths) || paths.length === 0) return undefined; + const extensionPaths: string[] = []; + for (const path of paths) { + if (typeof path !== "string" || path.length === 0 || !pathFlavor.isAbsolute(path)) return undefined; + extensionPaths.push(path); + } + return { noExtensions: record.noExtensions, extensionPaths }; +} + +// Plain argv elements for a child pi: spawned without a shell, so no quoting. +export function childPackageExtensionArgs(injection: ChildPackageInjection): string[] { + const args = injection.noExtensions ? ["--no-extensions"] : []; + for (const path of injection.extensionPaths) args.push("--extension", path); + return args; +} diff --git a/lib/gentle-shell-launcher.ts b/lib/gentle-shell-launcher.ts index 677c2a233..e125a04a0 100644 --- a/lib/gentle-shell-launcher.ts +++ b/lib/gentle-shell-launcher.ts @@ -1,4 +1,5 @@ -import { join, resolve as resolvePath } from "node:path"; +import { isAbsolute, join, resolve as resolvePath } from "node:path"; +import { CHILD_PACKAGE_INJECTION_ENV, encodeChildPackageInjection, type ChildPackageInjection } from "./child-package-injection.ts"; // The gentle-shell launcher: pure, side-effect-free functions over injected // env/fs/exec. `bin/gentle-shell.mjs` (T2) wires these into the real process, @@ -878,6 +879,10 @@ export interface BuildPiInvocationInput { baseEnv: Record; // The OS home behind userPiHome's conventional ~/.pi/agent fallback. homedir: string; + // The directory pi is spawned in, which pi resolves a relative -e path + // against. bin/gentle-shell.mjs spawns pi without a cwd, so this is the + // launcher's own process.cwd(). + cwd: string; } export interface PiInvocation { @@ -919,8 +924,15 @@ export interface PiInvocation { // - Not takeOver, with a declaration: no injection at all — the target // settings already load a gentle-pi the launcher accepts as-is (the // `--link` case with a pi-managed install matching this launcher). +// +// The two injecting cases also export CHILD_PACKAGE_INJECTION_ENV (#1690) so +// the subagent runner can give delegated children the same package. It holds +// only the launcher's own computed -e set; passthrough -e flags (the managed +// herdr extension, or one the user typed) are not part of it. Every other case +// removes an inherited value, so a nested launch never leaks a stale signal. export function buildPiInvocation(input: BuildPiInvocationInput): PiInvocation { const args = [...input.runtime.args]; + let childInjection: ChildPackageInjection | undefined; if (input.piSubcommand !== undefined) { // No injection at all: pi must see the bare subcommand as argv[0]. @@ -945,23 +957,36 @@ export function buildPiInvocation(input: BuildPiInvocationInput): PiInvocation { injected.add(input.packageRoot); args.push("-e", input.packageRoot); } - + // The argv dedupe above compares raw strings; the signal dedupes again + // after absolutizing, so a relative and an absolute spelling of the same + // file appear once, in first-occurrence order. + const signalPaths = new Set([...injected].map((path) => absoluteExtensionPath(path, input.cwd))); + childInjection = { noExtensions: true, extensionPaths: [...signalPaths] }; } else if (input.declaration === undefined) { args.push("-e", input.packageRoot); + childInjection = { noExtensions: false, extensionPaths: [absoluteExtensionPath(input.packageRoot, input.cwd)] }; } args.push(...input.passthrough); - return { - command: input.runtime.command, - args, - env: { - ...input.baseEnv, - PI_CODING_AGENT_DIR: input.home.dir, - GENTLE_PI_AGENT_HOME: input.home.dir, - [USER_PI_HOME_ENV]: userPiHome(input.baseEnv, input.homedir), - }, + const env: Record = { + ...input.baseEnv, + PI_CODING_AGENT_DIR: input.home.dir, + GENTLE_PI_AGENT_HOME: input.home.dir, + [USER_PI_HOME_ENV]: userPiHome(input.baseEnv, input.homedir), }; + if (childInjection === undefined) delete env[CHILD_PACKAGE_INJECTION_ENV]; + else env[CHILD_PACKAGE_INJECTION_ENV] = encodeChildPackageInjection(childInjection); + + return { command: input.runtime.command, args, env }; +} + +// pi resolves a relative -e path against its spawn cwd. Children may run +// elsewhere, so the signal carries the same file as an absolute path. Loose +// entries can be relative when the isolated or linked home comes from a +// relative env value. +function absoluteExtensionPath(path: string, cwd: string): string { + return isAbsolute(path) ? path : resolvePath(cwd, path); } // --- spawn planning ------------------------------------------------------------ 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..fbd594673 --- /dev/null +++ b/odd/tasks/fix-1690-standalone-child-package.md @@ -0,0 +1,122 @@ +# 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. +- [x] 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: delegated writer (musgrl7b-5-7p0z, continued as musgx2ah-6-dep2). One env var, `GENTLE_SHELL_CHILD_PACKAGE_INJECTION` = JSON `{version:1, noExtensions, extensionPaths}`, defined once in `lib/child-package-injection.ts` (encoder, a parser that never throws and requires absolute paths, and the child argv helper). Takeover sets the exact deduped `-e` set with `noExtensions: true`; no declaration sets `[packageRoot]`; declaration and pi subcommand delete any inherited value. Passthrough `-e` (herdr, user-typed) is deliberately excluded. `bin/gentle-shell.mjs` imports the generated `runtime/gentle-shell-launcher.mjs`, so the module was registered in `scripts/build-runtime-modules.mjs` and `scripts/verify-package-files.mjs`, and the runtime was regenerated. Evidence: launcher 213/213, module 6/6, bin 129/129 (the new bin test went RED with the assignment disabled), package-manifest 56/56; `build-runtime-modules --check` and `verify-package-files` pass; check-types shows no regressions. Commit: the T3 commit carrying this line. +- [ ] 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 + +- [ ] Optional PR1 polish (T2b review suggestions, non-blocking): (a) `tests/gentle-ai-child-guards.test.ts:90-94`: await the `tool_execution_start` handler before counting ledger entries; (b) `extensions/gentle-ai.ts:9686-9688`: no test asserts that `yolo.reset`/`reviewSidebar.reset` run in a child. Add one, or narrow the comment. + +- [ ] **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. +- T2b RDD review (native Claude Code session): `approved`, medium risk, one lens (review-reliability), candidate `54effec6` against base `f384d2a1`, no correction; authority burned (lineage `review-8c111c00f615d9c9`). Both T2 findings confirmed resolved. Two new non-blocking suggestions were tracked in "Pending follow-ups". +- 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/runtime/child-package-injection.mjs b/runtime/child-package-injection.mjs new file mode 100644 index 000000000..07c03a416 --- /dev/null +++ b/runtime/child-package-injection.mjs @@ -0,0 +1,63 @@ +// Generated by scripts/build-runtime-modules.mjs. Do not edit. +import { isAbsolute } from "node:path"; + +// #1690: the gentle-shell launcher injects the gentle-pi package into the +// parent pi with `-e` instead of a settings declaration, so delegated children +// would otherwise start without it. The launcher records the exact extension +// set it injected in this variable; the subagent runner forwards that set to +// every child. Kept free of launcher imports so the runner can load it cheaply. +export const CHILD_PACKAGE_INJECTION_ENV = "GENTLE_SHELL_CHILD_PACKAGE_INJECTION"; + +const CHILD_PACKAGE_INJECTION_VERSION = 1; + + + + + + + + + +export function encodeChildPackageInjection(value ) { + return JSON.stringify({ + version: CHILD_PACKAGE_INJECTION_VERSION, + noExtensions: value.noExtensions, + extensionPaths: value.extensionPaths, + }); +} + +// Returns undefined for an absent or invalid value and never throws: a bad +// signal must degrade to "no forwarding", not break a child launch. The path +// flavor is injectable so tests can check Windows paths on any platform. +export function parseChildPackageInjection( + env , + pathFlavor = { isAbsolute }, +) { + const raw = env[CHILD_PACKAGE_INJECTION_ENV]; + if (raw === undefined || raw.length === 0) return undefined; + let parsed ; + try { + parsed = JSON.parse(raw); + } catch { + return undefined; + } + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return undefined; + const record = parsed ; + if (record.version !== CHILD_PACKAGE_INJECTION_VERSION) return undefined; + if (typeof record.noExtensions !== "boolean") return undefined; + const paths = record.extensionPaths; + if (!Array.isArray(paths) || paths.length === 0) return undefined; + const extensionPaths = []; + for (const path of paths) { + if (typeof path !== "string" || path.length === 0 || !pathFlavor.isAbsolute(path)) return undefined; + extensionPaths.push(path); + } + return { noExtensions: record.noExtensions, extensionPaths }; +} + +// Plain argv elements for a child pi: spawned without a shell, so no quoting. +export function childPackageExtensionArgs(injection ) { + const args = injection.noExtensions ? ["--no-extensions"] : []; + for (const path of injection.extensionPaths) args.push("--extension", path); + return args; +} diff --git a/runtime/gentle-shell-launcher.mjs b/runtime/gentle-shell-launcher.mjs index f35e89af4..7e7000f08 100644 --- a/runtime/gentle-shell-launcher.mjs +++ b/runtime/gentle-shell-launcher.mjs @@ -1,5 +1,6 @@ // Generated by scripts/build-runtime-modules.mjs. Do not edit. -import { join, resolve as resolvePath } from "node:path"; +import { isAbsolute, join, resolve as resolvePath } from "node:path"; +import { CHILD_PACKAGE_INJECTION_ENV, encodeChildPackageInjection, } from "./child-package-injection.mjs"; // The gentle-shell launcher: pure, side-effect-free functions over injected // env/fs/exec. `bin/gentle-shell.mjs` (T2) wires these into the real process, @@ -883,6 +884,10 @@ export function discoverLooseExtensionEntries(dir , fs ) + + + + @@ -920,8 +925,15 @@ export function discoverLooseExtensionEntries(dir , fs ) // - Not takeOver, with a declaration: no injection at all — the target // settings already load a gentle-pi the launcher accepts as-is (the // `--link` case with a pi-managed install matching this launcher). +// +// The two injecting cases also export CHILD_PACKAGE_INJECTION_ENV (#1690) so +// the subagent runner can give delegated children the same package. It holds +// only the launcher's own computed -e set; passthrough -e flags (the managed +// herdr extension, or one the user typed) are not part of it. Every other case +// removes an inherited value, so a nested launch never leaks a stale signal. export function buildPiInvocation(input ) { const args = [...input.runtime.args]; + let childInjection ; if (input.piSubcommand !== undefined) { // No injection at all: pi must see the bare subcommand as argv[0]. @@ -946,23 +958,36 @@ export function buildPiInvocation(input ) { injected.add(input.packageRoot); args.push("-e", input.packageRoot); } - + // The argv dedupe above compares raw strings; the signal dedupes again + // after absolutizing, so a relative and an absolute spelling of the same + // file appear once, in first-occurrence order. + const signalPaths = new Set([...injected].map((path) => absoluteExtensionPath(path, input.cwd))); + childInjection = { noExtensions: true, extensionPaths: [...signalPaths] }; } else if (input.declaration === undefined) { args.push("-e", input.packageRoot); + childInjection = { noExtensions: false, extensionPaths: [absoluteExtensionPath(input.packageRoot, input.cwd)] }; } args.push(...input.passthrough); - return { - command: input.runtime.command, - args, - env: { - ...input.baseEnv, - PI_CODING_AGENT_DIR: input.home.dir, - GENTLE_PI_AGENT_HOME: input.home.dir, - [USER_PI_HOME_ENV]: userPiHome(input.baseEnv, input.homedir), - }, + const env = { + ...input.baseEnv, + PI_CODING_AGENT_DIR: input.home.dir, + GENTLE_PI_AGENT_HOME: input.home.dir, + [USER_PI_HOME_ENV]: userPiHome(input.baseEnv, input.homedir), }; + if (childInjection === undefined) delete env[CHILD_PACKAGE_INJECTION_ENV]; + else env[CHILD_PACKAGE_INJECTION_ENV] = encodeChildPackageInjection(childInjection); + + return { command: input.runtime.command, args, env }; +} + +// pi resolves a relative -e path against its spawn cwd. Children may run +// elsewhere, so the signal carries the same file as an absolute path. Loose +// entries can be relative when the isolated or linked home comes from a +// relative env value. +function absoluteExtensionPath(path , cwd ) { + return isAbsolute(path) ? path : resolvePath(cwd, path); } // --- spawn planning ------------------------------------------------------------ diff --git a/scripts/build-runtime-modules.mjs b/scripts/build-runtime-modules.mjs index 86b047e23..8dcdae7dc 100644 --- a/scripts/build-runtime-modules.mjs +++ b/scripts/build-runtime-modules.mjs @@ -13,6 +13,7 @@ const sources = [ "review-risk-assessment", "native-review-cli", "telemetry-trigger", + "child-package-injection", "gentle-shell-launcher", "gentle-shell-resume-hint", ]; diff --git a/scripts/verify-package-files.mjs b/scripts/verify-package-files.mjs index 1d1762123..8a01d9200 100644 --- a/scripts/verify-package-files.mjs +++ b/scripts/verify-package-files.mjs @@ -40,6 +40,7 @@ const requiredPaths = [ "extensions/gentle-ai.ts", "extensions/resume-hint.ts", "extensions/skill-registry.ts", + "lib/child-package-injection.ts", "lib/gentle-ai-binary.ts", "lib/gentle-shell-launcher.ts", "lib/gentle-shell-resume-hint.ts", @@ -50,6 +51,7 @@ const requiredPaths = [ "lib/review-relay-contract.ts", "lib/agent-assets.ts", "lib/telemetry-trigger.ts", + "runtime/child-package-injection.mjs", "runtime/gentle-ai-binary.mjs", "runtime/gentle-shell-launcher.mjs", "runtime/gentle-shell-resume-hint.mjs", diff --git a/tests/child-package-injection.test.ts b/tests/child-package-injection.test.ts new file mode 100644 index 000000000..ea1a208d2 --- /dev/null +++ b/tests/child-package-injection.test.ts @@ -0,0 +1,74 @@ +import assert from "node:assert/strict"; +import { posix, win32 } from "node:path"; +import test from "node:test"; +import { + CHILD_PACKAGE_INJECTION_ENV, + childPackageExtensionArgs, + encodeChildPackageInjection, + parseChildPackageInjection, +} from "../lib/child-package-injection.ts"; + +const envWith = (value: string | undefined) => ({ [CHILD_PACKAGE_INJECTION_ENV]: value }); + +test("the env name follows the GENTLE_SHELL_* launcher naming", () => { + assert.equal(CHILD_PACKAGE_INJECTION_ENV, "GENTLE_SHELL_CHILD_PACKAGE_INJECTION"); +}); + +test("encode and parse round-trip both launcher shapes", () => { + for (const value of [ + { noExtensions: false, extensionPaths: ["/pkg"] }, + { noExtensions: true, extensionPaths: ["/agent/npm/node_modules/other", "/agent/extensions/a.ts", "/pkg"] }, + ]) { + const encoded = encodeChildPackageInjection(value); + assert.deepEqual(JSON.parse(encoded), { version: 1, ...value }); + assert.deepEqual(parseChildPackageInjection(envWith(encoded)), value); + } +}); + +test("parse returns undefined when the value is absent or empty", () => { + assert.equal(parseChildPackageInjection({}), undefined); + assert.equal(parseChildPackageInjection(envWith(undefined)), undefined); + assert.equal(parseChildPackageInjection(envWith("")), undefined); +}); + +test("parse rejects malformed values without throwing", () => { + const rejected = [ + "{not json", + "null", + "[]", + "42", + JSON.stringify({ noExtensions: false, extensionPaths: ["/pkg"] }), + JSON.stringify({ version: 2, noExtensions: false, extensionPaths: ["/pkg"] }), + JSON.stringify({ version: "1", noExtensions: false, extensionPaths: ["/pkg"] }), + JSON.stringify({ version: 1, extensionPaths: ["/pkg"] }), + JSON.stringify({ version: 1, noExtensions: "false", extensionPaths: ["/pkg"] }), + JSON.stringify({ version: 1, noExtensions: false }), + JSON.stringify({ version: 1, noExtensions: false, extensionPaths: "/pkg" }), + JSON.stringify({ version: 1, noExtensions: false, extensionPaths: [] }), + JSON.stringify({ version: 1, noExtensions: false, extensionPaths: [42] }), + JSON.stringify({ version: 1, noExtensions: false, extensionPaths: [null] }), + JSON.stringify({ version: 1, noExtensions: false, extensionPaths: [""] }), + JSON.stringify({ version: 1, noExtensions: false, extensionPaths: ["pkg"] }), + JSON.stringify({ version: 1, noExtensions: true, extensionPaths: ["/pkg", "./extensions/a.ts"] }), + ]; + for (const value of rejected) { + assert.doesNotThrow(() => parseChildPackageInjection(envWith(value)), value); + assert.equal(parseChildPackageInjection(envWith(value)), undefined, value); + } +}); + +test("parse checks absoluteness with the injected path flavor, so Windows paths are accepted", () => { + const windows = encodeChildPackageInjection({ noExtensions: false, extensionPaths: ["C:\\x\\gentle-pi"] }); + assert.deepEqual(parseChildPackageInjection(envWith(windows), win32), { noExtensions: false, extensionPaths: ["C:\\x\\gentle-pi"] }); + assert.equal(parseChildPackageInjection(envWith(windows), posix), undefined); + const driveRelative = encodeChildPackageInjection({ noExtensions: false, extensionPaths: ["C:x"] }); + assert.equal(parseChildPackageInjection(envWith(driveRelative), win32), undefined); +}); + +test("childPackageExtensionArgs emits plain argv elements, with --no-extensions first for a takeover", () => { + assert.deepEqual(childPackageExtensionArgs({ noExtensions: false, extensionPaths: ["/pkg"] }), ["--extension", "/pkg"]); + assert.deepEqual( + childPackageExtensionArgs({ noExtensions: true, extensionPaths: ["/other dir/pkg", "C:\\x y\\a.ts", "/pkg"] }), + ["--no-extensions", "--extension", "/other dir/pkg", "--extension", "C:\\x y\\a.ts", "--extension", "/pkg"], + ); +}); 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/gentle-shell-bin.test.ts b/tests/gentle-shell-bin.test.ts index 5ff2475c9..33cdf72ed 100644 --- a/tests/gentle-shell-bin.test.ts +++ b/tests/gentle-shell-bin.test.ts @@ -40,7 +40,7 @@ test("Herdr activity is discoverable through the isolated launcher package", asy runtime: { kind: "path", command: "fake-pi", args: [] }, home: { mode: "isolated", source: "default", dir: "/fake/home" }, packageRoot, declaration: undefined, takeOver, otherPackagePaths: [], - passthrough: [], baseEnv: {}, homedir: "/fake", + passthrough: [], baseEnv: {}, homedir: "/fake", cwd: "/fake/cwd", }); assert.ok(invocation.args.includes(packageRoot)); assert.equal(invocation.env.PI_CODING_AGENT_DIR, "/fake/home"); @@ -169,6 +169,7 @@ function writePiScript(path: string, version: string, removeExitCode = 0) { " PI_CODING_AGENT_DIR: process.env.PI_CODING_AGENT_DIR,", " GENTLE_PI_AGENT_HOME: process.env.GENTLE_PI_AGENT_HOME,", " GENTLE_SHELL_USER_PI_HOME: process.env.GENTLE_SHELL_USER_PI_HOME,", + " GENTLE_SHELL_CHILD_PACKAGE_INJECTION: process.env.GENTLE_SHELL_CHILD_PACKAGE_INJECTION,", "}));", "process.exit(0);", "", @@ -659,6 +660,20 @@ test("forwarded args reach pi after the injected extension flags, in order", (t) assert.equal(payload.GENTLE_PI_AGENT_HOME, f.gentleShellHome); }); +// #1690: the spawned pi must carry the launcher's own -e set so the subagent +// runner can forward it to delegated children; a stale inherited value is replaced. +test("an isolated launch without a gentle-pi declaration signals its package injection to pi", (t) => { + const f = fixture(t); + const stale = JSON.stringify({ version: 1, noExtensions: true, extensionPaths: [join(f.root, "outer")] }); + for (const inherited of [undefined, stale]) { + const result = run({ ...f.env, GENTLE_SHELL_CHILD_PACKAGE_INJECTION: inherited }, ["--mode", "rpc"]); + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.deepEqual(payload.args.slice(0, 2), ["-e", packageRoot]); + assert.deepEqual(JSON.parse(payload.GENTLE_SHELL_CHILD_PACKAGE_INJECTION), { version: 1, noExtensions: false, extensionPaths: [packageRoot] }); + } +}); + test("an isolated launch keeps its own agent home and carries the user's original Pi home, even when nested", (t) => { const f = fixture(t); const base = { ...f.env }; diff --git a/tests/gentle-shell-launcher.test.ts b/tests/gentle-shell-launcher.test.ts index bc1ba77ad..a6ea4b563 100644 --- a/tests/gentle-shell-launcher.test.ts +++ b/tests/gentle-shell-launcher.test.ts @@ -1,8 +1,9 @@ import assert from "node:assert/strict"; import { readFileSync } from "node:fs"; -import { join } from "node:path"; +import { join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import test from "node:test"; +import { CHILD_PACKAGE_INJECTION_ENV, encodeChildPackageInjection, parseChildPackageInjection } from "../lib/child-package-injection.ts"; import { MIN_PI_VERSION, MIN_SETUP_GENTLE_AI_VERSION, @@ -1134,6 +1135,7 @@ test("package injection relies on Pi's -e resource discovery in isolated and tak passthrough: [], baseEnv: {}, homedir: "/home/u", + cwd: "/work", }); assert.deepEqual(built.args, takeOver ? ["--no-extensions", "-e", "/pkg"] : ["-e", "/pkg"]); } @@ -1153,6 +1155,7 @@ test("buildPiInvocation injects the launcher env into baseEnv", () => { passthrough: [], baseEnv: { PATH: "/usr/bin" }, homedir: "/home/u", + cwd: "/work", }); assert.deepEqual(built.env, { PATH: "/usr/bin", PI_CODING_AGENT_DIR: "/pi/agent", GENTLE_PI_AGENT_HOME: "/pi/agent", GENTLE_SHELL_USER_PI_HOME: join("/home/u", ".pi", "agent") }); }); @@ -1168,6 +1171,7 @@ test("buildPiInvocation skips injection when there is a declaration and no takeo passthrough: ["--mode", "rpc"], baseEnv: {}, homedir: "/home/u", + cwd: "/work", }); assert.deepEqual(built.command, "/usr/bin/pi"); assert.deepEqual(built.args, ["--mode", "rpc"]); @@ -1184,6 +1188,7 @@ test("buildPiInvocation adds the gentle-pi injection flags when there is no decl passthrough: ["--mode", "rpc"], baseEnv: {}, homedir: "/home/u", + cwd: "/work", }); assert.deepEqual(built.args, [ "-e", @@ -1205,6 +1210,7 @@ test("buildPiInvocation emits only runtime args and passthrough when a pi subcom piSubcommand: "install", baseEnv: {}, homedir: "/home/u", + cwd: "/work", }); assert.deepEqual(built.args, ["install", "npm:x"]); assert.equal(built.command, "/usr/bin/pi"); @@ -1222,6 +1228,7 @@ test("buildPiInvocation still injects the launcher env for a pi subcommand", () piSubcommand: "list", baseEnv: { PATH: "/usr/bin" }, homedir: "/home/u", + cwd: "/work", }); assert.deepEqual(built.env, { PATH: "/usr/bin", @@ -1243,6 +1250,7 @@ test("buildPiInvocation in link mode with a pi subcommand is exactly pi built.args[index - 1] === "-e"); assert.deepEqual(eFlags, [join("/agent", "npm", "node_modules", "some-other"), "/pkg"]); @@ -1480,6 +1498,127 @@ test("buildPiInvocation dedupes the launcher's own package root against an other ]); }); +// --- child package injection signal (#1690) ---------------------------------- +// +// The launcher exports exactly the -e set it computed itself, so the subagent +// runner can forward it to delegated children. Passthrough -e flags are not +// part of the signal. + +const injectionInput = { + runtime: { kind: "path" as const, command: "/usr/bin/pi", args: [] }, + home: isolatedHomeResolved, + packageRoot: "/pkg", + otherPackagePaths: [], + passthrough: ["-e", "/user/ext.ts", "--mode", "rpc"], + homedir: "/home/u", + cwd: "/work", +}; + +test("buildPiInvocation signals the packageRoot injection when there is no declaration", () => { + const built = buildPiInvocation({ ...injectionInput, declaration: undefined, takeOver: false, baseEnv: {} }); + assert.deepEqual(parseChildPackageInjection(built.env), { noExtensions: false, extensionPaths: ["/pkg"] }); +}); + +test("buildPiInvocation signals the exact deduped takeover -e set in order, without passthrough -e flags", () => { + const built = buildPiInvocation({ + ...injectionInput, + home: linkHome, + declaration: { kind: "path", dir: "/other/checkout" }, + takeOver: true, + otherPackagePaths: ["/agent/npm/node_modules/some-other", "/shared/dup.ts", "/pkg"], + looseExtensionEntries: ["/shared/dup.ts", "/agent/extensions/a.ts", "/agent/extensions/a.ts"], + baseEnv: {}, + }); + const launcherPaths = built.args.slice(0, built.args.indexOf("--mode") - 2).filter((_, index, all) => all[index - 1] === "-e"); + const signal = parseChildPackageInjection(built.env); + assert.deepEqual(signal, { + noExtensions: true, + // /pkg keeps its first (other-package) position, exactly as in argv. + extensionPaths: ["/agent/npm/node_modules/some-other", "/shared/dup.ts", "/pkg", "/agent/extensions/a.ts"], + }); + assert.deepEqual(signal?.extensionPaths, launcherPaths); +}); + +test("buildPiInvocation signals absolute paths resolved against the given cwd when a takeover receives a relative loose entry", () => { + const cwd = resolve("/elsewhere", "launch-dir"); + assert.notEqual(cwd, process.cwd()); + const built = buildPiInvocation({ + ...injectionInput, + cwd, + home: linkHome, + declaration: { kind: "npm" }, + takeOver: true, + looseExtensionEntries: [join("relative-home", "extensions", "a.ts")], + baseEnv: {}, + }); + assert.ok(built.args.includes(join("relative-home", "extensions", "a.ts")), "the -e argv itself is unchanged"); + assert.deepEqual(parseChildPackageInjection(built.env)?.extensionPaths, [resolve(cwd, "relative-home", "extensions", "a.ts"), "/pkg"]); +}); + +test("buildPiInvocation signals one entry for a file spelled both relative and absolute, while argv keeps both spellings", () => { + const cwd = resolve("/elsewhere", "launch-dir"); + const relative = join("relative-home", "extensions", "a.ts"); + const absolute = resolve(cwd, relative); + const built = buildPiInvocation({ + ...injectionInput, + cwd, + home: linkHome, + declaration: { kind: "npm" }, + takeOver: true, + otherPackagePaths: ["/agent/npm/node_modules/some-other", absolute], + looseExtensionEntries: [relative, "/agent/extensions/b.ts"], + baseEnv: {}, + }); + assert.deepEqual(built.args.slice(0, built.args.indexOf("--mode") - 2), [ + "--no-extensions", + "-e", + "/agent/npm/node_modules/some-other", + "-e", + absolute, + "-e", + relative, + "-e", + "/agent/extensions/b.ts", + "-e", + "/pkg", + ]); + assert.deepEqual(parseChildPackageInjection(built.env)?.extensionPaths, [ + "/agent/npm/node_modules/some-other", + absolute, + "/agent/extensions/b.ts", + "/pkg", + ]); +}); + +test("buildPiInvocation does not signal when settings already declare gentle-pi, and drops an inherited signal", () => { + const stale = encodeChildPackageInjection({ noExtensions: false, extensionPaths: ["/outer/pkg"] }); + for (const baseEnv of [{}, { [CHILD_PACKAGE_INJECTION_ENV]: stale }]) { + const built = buildPiInvocation({ ...injectionInput, home: linkHome, declaration: { kind: "npm" }, takeOver: false, baseEnv }); + assert.equal(Object.hasOwn(built.env, CHILD_PACKAGE_INJECTION_ENV), false); + } +}); + +test("buildPiInvocation does not signal for a pi subcommand, and drops an inherited signal", () => { + const stale = encodeChildPackageInjection({ noExtensions: false, extensionPaths: ["/outer/pkg"] }); + for (const takeOver of [false, true]) { + const built = buildPiInvocation({ + ...injectionInput, + declaration: undefined, + takeOver, + passthrough: ["list"], + piSubcommand: "list", + baseEnv: { [CHILD_PACKAGE_INJECTION_ENV]: stale }, + }); + assert.equal(Object.hasOwn(built.env, CHILD_PACKAGE_INJECTION_ENV), false); + } +}); + +test("buildPiInvocation replaces an inherited signal with its own injection", () => { + const stale = encodeChildPackageInjection({ noExtensions: true, extensionPaths: ["/outer/pkg"] }); + const built = buildPiInvocation({ ...injectionInput, declaration: undefined, takeOver: false, baseEnv: { [CHILD_PACKAGE_INJECTION_ENV]: stale } }); + assert.deepEqual(parseChildPackageInjection(built.env), { noExtensions: false, extensionPaths: ["/pkg"] }); +}); + // --- discoverLooseExtensionEntries ------------------------------------------- // // Pure mirror of pi's own discoverExtensionsInDir (packages/coding-agent/src/ 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); +});