diff --git a/docs/workflow/bash-worktree-cwd-record.md b/docs/workflow/bash-worktree-cwd-record.md index c1d7992..ba18906 100644 --- a/docs/workflow/bash-worktree-cwd-record.md +++ b/docs/workflow/bash-worktree-cwd-record.md @@ -3,8 +3,8 @@ ## Status - Status: active -- Current phase: 7. Independent review -- Next safe action: obtain standards and specification review of the pinned product diff. +- Current phase: 6. Verify remediated worktree +- Next safe action: commit the accepted review remediations and rerun the full repository test suite. ## Request and scope @@ -46,6 +46,7 @@ | Workflow setup and research | parent Pi session `01a06e08-8d07-76a7-a116-d7e3baf55b87` | 15 minutes | workflow record only | Before: clean `main` at `7e7cfdb`; after: clean feature worktree at `5f853dc` | Complete | | Initial dependency-plan implementation | parent Pi session `01a06e08-8d07-76a7-a116-d7e3baf55b87` | 15 minutes | `package.json`, `bun.lock`, focused test, workflow record | Before: clean feature worktree at `9d68d5f`; after: Pi 0.85.0 upgrade restored; focused red test retained unstaged | Paused and replanned after upstream packaging blocker | | Local override implementation | parent Pi session `01a06e08-8d07-76a7-a116-d7e3baf55b87` | 15 minutes | `extensions/bash-cwd/**`, `scripts/bash-cwd.test.mjs` removal, `package.json`, `scripts/package-workspaces.test.mjs`, `README.md`, `docs/npm-packages.md`, `docs/todos.md`, `docs/workflow/skill-led-development-workflow-plan.md`, workflow record | Before: feature worktree at `3d68f7c` with inspected product changes; after: clean product commit `328faa0` | Complete | +| Review remediation cycle 1 | parent Pi session `01a06e08-8d07-76a7-a116-d7e3baf55b87` | 15 minutes | `extensions/bash-cwd/index.ts`, `extensions/bash-cwd/index.test.ts`, `extensions/bash-cwd/README.md`, workflow record | Before: clean review pin `3cf354b`; after: focused tests and typecheck pass, remediation commit pending | In progress | ## Verification matrix @@ -53,7 +54,7 @@ | --- | --- | --- | --- | | Source/cache preflight | `git status --short --branch`; `opensrc path ...` | 0 | Clean source; Pi 0.84.4 and 0.85.0 source available locally. | | Focused cwd reproduction | inline Node harness against Pi 0.84.4 | 0 (asserted buggy outcome) | Both hidden-worktree requests observed the primary checkout; a missing requested cwd still executed a sentinel command. | -| Focused cwd regression | `bun run --filter '@jbcode/pi-bash-cwd' test` | 0 | Three tests pass: explicit absolute hidden-worktree cwd, relative nested-package cwd, omitted cwd/session environment, schema compatibility, missing cwd sentinel, and missing context. | +| Focused cwd regression | `bun run --filter '@jbcode/pi-bash-cwd' test` | 0 | Four tests pass: explicit absolute hidden-worktree cwd, relative nested-package cwd, omitted cwd/session environment, configured shell path and command prefix, schema compatibility, missing cwd sentinel, and missing context. | | Package contract | `bun run test:packages`; `bun pm pack --destination /tmp --ignore-scripts --quiet` | 0 | Workspace inventory and package contents pass; package tarball created. | | Typecheck | `bun run typecheck` | 0 | Repository TypeScript check passes. | | Full tests | `bun run test` | 0 | The complete repository test chain passed in background task `bg_jdbm_mtnfe0ie_1`; Node reported 305/305 in the consolidated suite and Vitest reported 22/22 file-search tests. | @@ -67,10 +68,10 @@ - Exact diff command: `git diff --binary --full-index --find-renames 7e7cfdb5cb0e1a1218b9a2c8250267d5099637fc...328faa03e014bb7d29296d520297d05f0981fb33 --` - SHA-256: `b3b1ee7bfc70655a1324f6da943e2192987693d62495912cc866151b58d1c1a7` - Mutator identities: parent Pi session `01a06e08-8d07-76a7-a116-d7e3baf55b87` -- Standards reviewer identity, digest confirmation, and verdict: pending -- Specification reviewer identity, digest confirmation, and verdict: pending -- Findings and remediation cycles: Pi 0.85.0 includes a separate upstream `ctx.cwd` correction but its public package imports undeclared `@earendil-works/pi-server`, causing `ERR_MODULE_NOT_FOUND`; the user selected a repository-owned override instead. During implementation, a direct live call with an extra `cwd` field still printed the primary checkout, proving Pi's schema/executor silently ignore the requested per-call directory and the first local design addressed the wrong seam. Current uncommitted package/test/docs changes were inspected; further mutation is paused for the corrected explicit-argument plan. -- Before/after reviewer Git evidence: pending +- Standards reviewer identity, digest confirmation, and verdict: `bash-cwd-standards`, scout child session `01a06e2f-665e-7acc-a2cd-98f976a13b43`; digest confirmed; FAIL on first review. +- Specification reviewer identity, digest confirmation, and verdict: `bash-cwd-specification`, scout child session `01a06e2f-b262-77f1-9aef-d5c964cb33b6`, subagent `8ae886b2`; digest confirmed; FAIL on first review. +- Findings and remediation cycles: Pi 0.85.0 includes a separate upstream `ctx.cwd` correction but its public package imports undeclared `@earendil-works/pi-server`, causing `ERR_MODULE_NOT_FOUND`; the user selected a repository-owned override instead. During implementation, a direct live call with an extra `cwd` field still printed the primary checkout, proving Pi's schema/executor silently ignore the requested per-call directory and the first local design addressed the wrong seam. Review cycle 1 accepted three findings: preserve configured `shellPath` and `shellCommandPrefix` when recreating Pi's built-in definition; make retirement contingent on equivalent validated per-call cwd; and label npm installation as post-publication. The parent implementer loaded trusted Pi settings at execution, added a realistic custom-shell/prefix regression, and corrected package guidance. +- Before/after reviewer Git evidence: both reviewers reported clean `feature/honor-bash-worktree-cwd` state, product HEAD `328faa0`, product tree `6b858045b91298c91f55aedd848c3a02432edafb`, and the complete changed-path set before and after their read-only reviews; current review-pin HEAD remained `3cf354b`. ## Integration and rollback @@ -83,5 +84,5 @@ ## Checkpoints and blockers - Last committed valid checkpoint: product commit `328faa0` -- Blockers: standards and specification review verdicts are pending. -- Exact next safe action: commit this review pin, then launch two independent read-only reviewers against the recorded digest. +- Blockers: full post-remediation verification and fresh reviews are pending. +- Exact next safe action: commit remediation cycle 1, rerun `bun run test`, and pin a fresh product digest. diff --git a/extensions/bash-cwd/README.md b/extensions/bash-cwd/README.md index d50f4cd..f03b987 100644 --- a/extensions/bash-cwd/README.md +++ b/extensions/bash-cwd/README.md @@ -4,15 +4,15 @@ Pi extension that adds an optional `cwd` argument to the built-in `bash` tool. A The override delegates execution and rendering to Pi's built-in Bash implementation. Existing `command` and `timeout` calls remain compatible. -## Install +## Try from this checkout ```bash -pi install npm:@jbcode/pi-bash-cwd +pi -e ./extensions/bash-cwd/index.ts ``` -Restart Pi or run `/reload` after installation. Pi warns that the extension replaces the built-in `bash` tool; that warning is expected. +Pi warns that the extension replaces the built-in `bash` tool; that warning is expected. The package is not published yet. After publication, install it with `pi install npm:@jbcode/pi-bash-cwd` and restart Pi or run `/reload`. -Use this package with Pi 0.84.x. Remove it after moving to a Pi release that honors `ctx.cwd` and can be imported by extensions without undeclared runtime dependencies. +Use this package with Pi 0.84.x. Remove it only after moving to a Pi release that exposes equivalent validated per-call `cwd` behavior and can be imported by extensions without undeclared runtime dependencies. ## Verify diff --git a/extensions/bash-cwd/index.test.ts b/extensions/bash-cwd/index.test.ts index e678da9..6631101 100644 --- a/extensions/bash-cwd/index.test.ts +++ b/extensions/bash-cwd/index.test.ts @@ -1,6 +1,13 @@ import assert from "node:assert/strict"; import { execFileSync } from "node:child_process"; -import { access, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { + access, + chmod, + mkdir, + mkdtemp, + rm, + writeFile, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import test from "node:test"; @@ -29,7 +36,7 @@ async function executeIn( cwd?: string, ) { const params = cwd === undefined ? { command } : { command, cwd }; - return createContextualBashTool().execute( + return createContextualBashTool({}).execute( "bash-cwd-test", params, undefined, @@ -91,14 +98,55 @@ test("bash honors hidden-worktree and nested-package cwd arguments", async (t) = await assert.rejects(access(sentinel)); }); +test("bash preserves configured command prefixes and shell paths", async (t) => { + const directory = await mkdtemp(join(tmpdir(), "pi-bash-cwd-options-")); + t.after(() => rm(directory, { recursive: true, force: true })); + + const shellMarker = join(directory, "custom-shell-used"); + const shell = join(directory, "custom-shell"); + await writeFile( + shell, + `#!/bin/sh\nprintf used > "${shellMarker}"\nexec /bin/bash "$@"\n`, + ); + await chmod(shell, 0o755); + + await mkdir(join(directory, ".pi")); + await writeFile( + join(directory, ".pi", "settings.json"), + JSON.stringify({ + shellCommandPrefix: "printf prefix-", + shellPath: shell, + }), + ); + + const tool = createContextualBashTool(); + const result = await tool.execute( + "bash-cwd-options-test", + { command: "printf command", cwd: directory }, + undefined, + undefined, + { + cwd: directory, + sessionManager: { + getSessionId: () => "bash-cwd-test-session", + getSessionFile: () => undefined, + }, + isProjectTrusted: () => true, + } as ExtensionContext, + ); + + assert.equal(getOutput(result), "prefix-command"); + await access(shellMarker); +}); + test("bash exposes cwd without changing existing arguments", () => { - const schema = createContextualBashTool().parameters; + const schema = createContextualBashTool({}).parameters; assert.deepEqual(Object.keys(schema.properties), ["command", "cwd", "timeout"]); assert.deepEqual(schema.required, ["command"]); }); test("bash fails before execution when Pi omits the tool context", async () => { - const tool = createContextualBashTool(); + const tool = createContextualBashTool({}); await assert.rejects( tool.execute( "bash-cwd-test", diff --git a/extensions/bash-cwd/index.ts b/extensions/bash-cwd/index.ts index c7f4084..d51f4ce 100644 --- a/extensions/bash-cwd/index.ts +++ b/extensions/bash-cwd/index.ts @@ -3,6 +3,9 @@ import { resolve } from "node:path"; import { createBashToolDefinition, + getAgentDir, + SettingsManager, + type BashToolOptions, type ExtensionAPI, type ExtensionContext, } from "@earendil-works/pi-coding-agent"; @@ -52,8 +55,18 @@ async function resolveWorkingDirectory( return cwd; } -export function createContextualBashTool() { - const baseTool = createBashToolDefinition(process.cwd()); +function loadBashOptions(ctx: ExtensionContext): BashToolOptions { + const settings = SettingsManager.create(ctx.cwd, getAgentDir(), { + projectTrusted: ctx.isProjectTrusted(), + }); + return { + commandPrefix: settings.getShellCommandPrefix(), + shellPath: settings.getShellPath(), + }; +} + +export function createContextualBashTool(options?: BashToolOptions) { + const baseTool = createBashToolDefinition(process.cwd(), options); return { ...baseTool, @@ -72,7 +85,10 @@ export function createContextualBashTool() { } const cwd = await resolveWorkingDirectory(ctx.cwd, params.cwd); - const contextualTool = createBashToolDefinition(cwd); + const contextualTool = createBashToolDefinition( + cwd, + options ?? loadBashOptions(ctx), + ); return contextualTool.execute( toolCallId, { command: params.command, timeout: params.timeout },