fix(bash): preserve configured shell behavior
This commit is contained in:
@@ -3,8 +3,8 @@
|
|||||||
## Status
|
## Status
|
||||||
|
|
||||||
- Status: active
|
- Status: active
|
||||||
- Current phase: 7. Independent review
|
- Current phase: 6. Verify remediated worktree
|
||||||
- Next safe action: obtain standards and specification review of the pinned product diff.
|
- Next safe action: commit the accepted review remediations and rerun the full repository test suite.
|
||||||
|
|
||||||
## Request and scope
|
## 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 |
|
| 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 |
|
| 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 |
|
| 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
|
## 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. |
|
| 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 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. |
|
| 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. |
|
| 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. |
|
| 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 --`
|
- Exact diff command: `git diff --binary --full-index --find-renames 7e7cfdb5cb0e1a1218b9a2c8250267d5099637fc...328faa03e014bb7d29296d520297d05f0981fb33 --`
|
||||||
- SHA-256: `b3b1ee7bfc70655a1324f6da943e2192987693d62495912cc866151b58d1c1a7`
|
- SHA-256: `b3b1ee7bfc70655a1324f6da943e2192987693d62495912cc866151b58d1c1a7`
|
||||||
- Mutator identities: parent Pi session `01a06e08-8d07-76a7-a116-d7e3baf55b87`
|
- Mutator identities: parent Pi session `01a06e08-8d07-76a7-a116-d7e3baf55b87`
|
||||||
- Standards reviewer identity, digest confirmation, and verdict: 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: pending
|
- 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. Current uncommitted package/test/docs changes were inspected; further mutation is paused for the corrected explicit-argument plan.
|
- 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: pending
|
- 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
|
## Integration and rollback
|
||||||
|
|
||||||
@@ -83,5 +84,5 @@
|
|||||||
## Checkpoints and blockers
|
## Checkpoints and blockers
|
||||||
|
|
||||||
- Last committed valid checkpoint: product commit `328faa0`
|
- Last committed valid checkpoint: product commit `328faa0`
|
||||||
- Blockers: standards and specification review verdicts are pending.
|
- Blockers: full post-remediation verification and fresh reviews are pending.
|
||||||
- Exact next safe action: commit this review pin, then launch two independent read-only reviewers against the recorded digest.
|
- Exact next safe action: commit remediation cycle 1, rerun `bun run test`, and pin a fresh product digest.
|
||||||
|
|||||||
@@ -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.
|
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
|
```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
|
## Verify
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,13 @@
|
|||||||
import assert from "node:assert/strict";
|
import assert from "node:assert/strict";
|
||||||
import { execFileSync } from "node:child_process";
|
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 { tmpdir } from "node:os";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
import test from "node:test";
|
import test from "node:test";
|
||||||
@@ -29,7 +36,7 @@ async function executeIn(
|
|||||||
cwd?: string,
|
cwd?: string,
|
||||||
) {
|
) {
|
||||||
const params = cwd === undefined ? { command } : { command, cwd };
|
const params = cwd === undefined ? { command } : { command, cwd };
|
||||||
return createContextualBashTool().execute(
|
return createContextualBashTool({}).execute(
|
||||||
"bash-cwd-test",
|
"bash-cwd-test",
|
||||||
params,
|
params,
|
||||||
undefined,
|
undefined,
|
||||||
@@ -91,14 +98,55 @@ test("bash honors hidden-worktree and nested-package cwd arguments", async (t) =
|
|||||||
await assert.rejects(access(sentinel));
|
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", () => {
|
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(Object.keys(schema.properties), ["command", "cwd", "timeout"]);
|
||||||
assert.deepEqual(schema.required, ["command"]);
|
assert.deepEqual(schema.required, ["command"]);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("bash fails before execution when Pi omits the tool context", async () => {
|
test("bash fails before execution when Pi omits the tool context", async () => {
|
||||||
const tool = createContextualBashTool();
|
const tool = createContextualBashTool({});
|
||||||
await assert.rejects(
|
await assert.rejects(
|
||||||
tool.execute(
|
tool.execute(
|
||||||
"bash-cwd-test",
|
"bash-cwd-test",
|
||||||
|
|||||||
@@ -3,6 +3,9 @@ import { resolve } from "node:path";
|
|||||||
|
|
||||||
import {
|
import {
|
||||||
createBashToolDefinition,
|
createBashToolDefinition,
|
||||||
|
getAgentDir,
|
||||||
|
SettingsManager,
|
||||||
|
type BashToolOptions,
|
||||||
type ExtensionAPI,
|
type ExtensionAPI,
|
||||||
type ExtensionContext,
|
type ExtensionContext,
|
||||||
} from "@earendil-works/pi-coding-agent";
|
} from "@earendil-works/pi-coding-agent";
|
||||||
@@ -52,8 +55,18 @@ async function resolveWorkingDirectory(
|
|||||||
return cwd;
|
return cwd;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function createContextualBashTool() {
|
function loadBashOptions(ctx: ExtensionContext): BashToolOptions {
|
||||||
const baseTool = createBashToolDefinition(process.cwd());
|
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 {
|
return {
|
||||||
...baseTool,
|
...baseTool,
|
||||||
@@ -72,7 +85,10 @@ export function createContextualBashTool() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const cwd = await resolveWorkingDirectory(ctx.cwd, params.cwd);
|
const cwd = await resolveWorkingDirectory(ctx.cwd, params.cwd);
|
||||||
const contextualTool = createBashToolDefinition(cwd);
|
const contextualTool = createBashToolDefinition(
|
||||||
|
cwd,
|
||||||
|
options ?? loadBashOptions(ctx),
|
||||||
|
);
|
||||||
return contextualTool.execute(
|
return contextualTool.execute(
|
||||||
toolCallId,
|
toolCallId,
|
||||||
{ command: params.command, timeout: params.timeout },
|
{ command: params.command, timeout: params.timeout },
|
||||||
|
|||||||
Reference in New Issue
Block a user