feat(coding-agent/tools): enabled shebang files to be auto-marked executable
- Added a `madeExecutable` result field to `WriteToolDetails` to surface executable changes. - Implemented `maybeMarkExecutableForShebang` to chmod shebang files executable while preserving existing mode bits and swallowing chmod errors. - Updated write flow and renderer output to return and display when a file was auto-marked executable.
This commit is contained in:
@@ -70,6 +70,8 @@ export type WriteToolInput = z.infer<typeof writeSchema>;
|
||||
export interface WriteToolDetails {
|
||||
diagnostics?: FileDiagnosticsResult;
|
||||
meta?: OutputMeta;
|
||||
/** Set when the file was auto-chmod'd because content begins with a `#!` shebang. */
|
||||
madeExecutable?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -103,6 +105,28 @@ function appendNoteToResult(result: AgentToolResult<WriteToolDetails>, note: str
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* If `content` begins with a `#!` shebang, ensure the file is executable.
|
||||
*
|
||||
* Mirrors `chmod a+x` (adds user/group/other execute bits to existing mode).
|
||||
* Errors are swallowed: chmod failure (e.g. Windows ACL, read-only mount)
|
||||
* MUST NOT fail an otherwise successful write. Returns whether the mode
|
||||
* actually changed so the caller can surface a note.
|
||||
*/
|
||||
async function maybeMarkExecutableForShebang(absolutePath: string, content: string): Promise<boolean> {
|
||||
if (!content.startsWith("#!")) return false;
|
||||
try {
|
||||
const stat = await fs.stat(absolutePath);
|
||||
const currentMode = stat.mode & 0o7777;
|
||||
const newMode = currentMode | 0o111;
|
||||
if (newMode === currentMode) return false;
|
||||
await fs.chmod(absolutePath, newMode);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
// Tool Class
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
@@ -772,6 +796,7 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
|
||||
const diagnostics = await this.#writethrough(absolutePath, cleanContent, signal, undefined, batchRequest);
|
||||
invalidateFsScanAfterWrite(absolutePath);
|
||||
const madeExecutable = await maybeMarkExecutableForShebang(absolutePath, cleanContent);
|
||||
|
||||
const displayPath = formatPathRelativeToCwd(absolutePath, this.session.cwd);
|
||||
let resultText = `Successfully wrote ${cleanContent.length} bytes to ${displayPath}`;
|
||||
@@ -781,7 +806,7 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
if (!diagnostics) {
|
||||
return {
|
||||
content: [{ type: "text", text: resultText }],
|
||||
details: {},
|
||||
details: { madeExecutable: madeExecutable || undefined },
|
||||
};
|
||||
}
|
||||
|
||||
@@ -789,6 +814,7 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
content: [{ type: "text", text: resultText }],
|
||||
details: {
|
||||
diagnostics,
|
||||
madeExecutable: madeExecutable || undefined,
|
||||
meta: outputMeta()
|
||||
.diagnostics(diagnostics.summary, diagnostics.messages ?? [])
|
||||
.get(),
|
||||
@@ -915,13 +941,16 @@ export const writeToolRenderer = {
|
||||
const pathDisplay = filePath ? uiTheme.fg("accent", filePath) : uiTheme.fg("toolOutput", "…");
|
||||
const lineCount = countLines(fileContent);
|
||||
const lineSuffix = formatLineCountSuffix(lineCount, uiTheme);
|
||||
const execSuffix = result.details?.madeExecutable
|
||||
? `${uiTheme.fg("dim", " · ")}${uiTheme.fg("success", "made executable!")}`
|
||||
: "";
|
||||
|
||||
// Build header with status icon
|
||||
const header = renderStatusLine(
|
||||
{
|
||||
icon: "success",
|
||||
title: "Write",
|
||||
description: `${langIcon} ${pathDisplay}${lineSuffix}`,
|
||||
description: `${langIcon} ${pathDisplay}${lineSuffix}${execSuffix}`,
|
||||
},
|
||||
uiTheme,
|
||||
);
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
|
||||
import { WriteTool } from "@oh-my-pi/pi-coding-agent/tools/write";
|
||||
|
||||
function createSession(cwd: string): ToolSession {
|
||||
return {
|
||||
cwd,
|
||||
hasUI: false,
|
||||
getSessionFile: () => path.join(cwd, "session.jsonl"),
|
||||
getSessionSpawns: () => "*",
|
||||
getArtifactsDir: () => path.join(cwd, "artifacts"),
|
||||
allocateOutputArtifact: async () => ({ id: "artifact-1", path: path.join(cwd, "artifact-1.log") }),
|
||||
settings: Settings.isolated(),
|
||||
enableLsp: false,
|
||||
};
|
||||
}
|
||||
|
||||
function resultText(result: { content: { type: string; text?: string }[] }): string {
|
||||
return result.content
|
||||
.filter((b): b is { type: "text"; text: string } => b.type === "text" && typeof b.text === "string")
|
||||
.map(b => b.text)
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
function details(result: { details?: { madeExecutable?: boolean } }): { madeExecutable?: boolean } {
|
||||
return result.details ?? {};
|
||||
}
|
||||
|
||||
describe("write tool shebang chmod", () => {
|
||||
let tmpDir: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
await Settings.init({ inMemory: true });
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "write-shebang-test-"));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("marks files starting with #! as executable and flags the result", async () => {
|
||||
const filePath = path.join(tmpDir, "run.sh");
|
||||
const tool = new WriteTool(createSession(tmpDir));
|
||||
|
||||
const result = await tool.execute("call-1", {
|
||||
path: filePath,
|
||||
content: "#!/bin/sh\necho hi\n",
|
||||
});
|
||||
|
||||
const stat = await fs.stat(filePath);
|
||||
// All three execute bits flipped on (chmod a+x semantics).
|
||||
expect(stat.mode & 0o111).toBe(0o111);
|
||||
// Notice surfaces on details, not in the model-facing text.
|
||||
expect(details(result).madeExecutable).toBe(true);
|
||||
expect(resultText(result)).not.toContain("executable");
|
||||
});
|
||||
|
||||
it("does not chmod files without a shebang", async () => {
|
||||
const filePath = path.join(tmpDir, "data.txt");
|
||||
const tool = new WriteTool(createSession(tmpDir));
|
||||
|
||||
const result = await tool.execute("call-2", {
|
||||
path: filePath,
|
||||
content: "no shebang here\n",
|
||||
});
|
||||
|
||||
const stat = await fs.stat(filePath);
|
||||
expect(stat.mode & 0o111).toBe(0);
|
||||
expect(details(result).madeExecutable).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not re-flag when file is already executable", async () => {
|
||||
const filePath = path.join(tmpDir, "preexec.sh");
|
||||
await fs.writeFile(filePath, "#!/bin/sh\nold\n");
|
||||
await fs.chmod(filePath, 0o755);
|
||||
|
||||
const tool = new WriteTool(createSession(tmpDir));
|
||||
const result = await tool.execute("call-3", {
|
||||
path: filePath,
|
||||
content: "#!/usr/bin/env python3\nprint('hi')\n",
|
||||
});
|
||||
|
||||
const stat = await fs.stat(filePath);
|
||||
expect(stat.mode & 0o111).toBe(0o111);
|
||||
// Mode didn't change, so no flag.
|
||||
expect(details(result).madeExecutable).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user