fix(coding-agent): closed vault write approval bypass and fixed interaction tools
vault writes now rated write-tier and plan-mode enforced; .tar.gz rewrites keep gzip, are atomic, and write through symlinks; CRLF conflict detection works; conflict twins only invalidated when truly stale; ask discloses timeout auto-selection in result and transcript; todo rejects duplicate ids and stops persisting half-applied batches; auto-generated guard validates against mtime+size; ACP writes run post-write bookkeeping; irc errors set isError.
This commit is contained in:
@@ -59,6 +59,8 @@ export interface QuestionResult {
|
||||
multi: boolean;
|
||||
selectedOptions: string[];
|
||||
customInput?: string;
|
||||
/** True when the answer was auto-selected because the dialog timed out. */
|
||||
timedOut?: boolean;
|
||||
}
|
||||
|
||||
export interface AskToolDetails {
|
||||
@@ -67,6 +69,8 @@ export interface AskToolDetails {
|
||||
multi?: boolean;
|
||||
selectedOptions?: string[];
|
||||
customInput?: string;
|
||||
/** True when the answer was auto-selected because the dialog timed out. */
|
||||
timedOut?: boolean;
|
||||
/** Multi-part question mode */
|
||||
results?: QuestionResult[];
|
||||
}
|
||||
@@ -94,6 +98,10 @@ function toSelectOption(option: AskOption, label = option.label): ExtensionUISel
|
||||
|
||||
const OTHER_OPTION = "Other (type your own)";
|
||||
const RECOMMENDED_SUFFIX = " (Recommended)";
|
||||
// Window after the timeout deadline within which an `undefined` selection is
|
||||
// attributed to a UI-enforced timeout (for surfaces that close the dialog at
|
||||
// the deadline but never invoke `onTimeout`). Cancels beyond it are user Esc.
|
||||
const TIMEOUT_DETECTION_TOLERANCE_MS = 1_000;
|
||||
|
||||
function getDoneOptionLabel(): string {
|
||||
return `${theme.symbol("tool.ask")} Done selecting`;
|
||||
@@ -230,7 +238,12 @@ async function askSingleQuestion(
|
||||
? await untilAborted(signal, () => ui.select(prompt, optionsToShow, dialogOptions))
|
||||
: await ui.select(prompt, optionsToShow, dialogOptions);
|
||||
if (!timeoutTriggered && choice === undefined && typeof timeout === "number") {
|
||||
timeoutTriggered = Date.now() - startMs >= timeout;
|
||||
// Fallback for UI surfaces that enforce `timeout` without invoking
|
||||
// `onTimeout`: their auto-cancel resolves right at the deadline. A
|
||||
// cancel arriving well past the deadline is a deliberate user Esc on
|
||||
// a surface that kept the dialog open — keep treating it as a cancel.
|
||||
const elapsed = Date.now() - startMs;
|
||||
timeoutTriggered = elapsed >= timeout && elapsed <= timeout + TIMEOUT_DETECTION_TOLERANCE_MS;
|
||||
}
|
||||
return { choice, timedOut: timeoutTriggered, navigation: navigationAction };
|
||||
};
|
||||
@@ -380,9 +393,10 @@ function formatQuestionResult(result: QuestionResult): string {
|
||||
return `${result.id}: "${result.customInput}"`;
|
||||
}
|
||||
if (result.selectedOptions.length > 0) {
|
||||
const suffix = result.timedOut ? " (auto-selected after timeout)" : "";
|
||||
return result.multi
|
||||
? `${result.id}: [${result.selectedOptions.join(", ")}]`
|
||||
: `${result.id}: ${result.selectedOptions[0]}`;
|
||||
? `${result.id}: [${result.selectedOptions.join(", ")}]${suffix}`
|
||||
: `${result.id}: ${result.selectedOptions[0]}${suffix}`;
|
||||
}
|
||||
return `${result.id}: (cancelled)`;
|
||||
}
|
||||
@@ -519,13 +533,15 @@ export class AskTool implements AgentTool<typeof askSchema, AskToolDetails> {
|
||||
multi: q.multi ?? false,
|
||||
selectedOptions,
|
||||
customInput,
|
||||
timedOut: timedOut || undefined,
|
||||
};
|
||||
|
||||
const responseParts: string[] = [];
|
||||
if (selectedOptions.length > 0) {
|
||||
responseParts.push(
|
||||
q.multi ? `User selected: ${selectedOptions.join(", ")}` : `User selected: ${selectedOptions[0]}`,
|
||||
);
|
||||
const selectedText = q.multi
|
||||
? `User selected: ${selectedOptions.join(", ")}`
|
||||
: `User selected: ${selectedOptions[0]}`;
|
||||
responseParts.push(timedOut ? `${selectedText} (auto-selected after timeout)` : selectedText);
|
||||
}
|
||||
if (customInput !== undefined) {
|
||||
responseParts.push(
|
||||
@@ -573,6 +589,7 @@ export class AskTool implements AgentTool<typeof askSchema, AskToolDetails> {
|
||||
multi: q.multi ?? false,
|
||||
selectedOptions,
|
||||
customInput,
|
||||
timedOut: timedOut || undefined,
|
||||
};
|
||||
|
||||
if (navAction === "back") {
|
||||
@@ -828,9 +845,14 @@ export const askToolRenderer = {
|
||||
const dSelected = details.selectedOptions;
|
||||
const dMulti = details.multi;
|
||||
const dCustom = details.customInput;
|
||||
const dTimedOut = details.timedOut;
|
||||
return framedBlock(uiTheme, width => {
|
||||
const bodyLines = md(question, width);
|
||||
bodyLines.push(...renderAnswerOptionLines(uiTheme, mdTheme, dOptions, dSelected, dMulti, dCustom));
|
||||
if (dTimedOut) {
|
||||
// Distinguish auto-selection from a real user choice in the transcript.
|
||||
bodyLines.push(uiTheme.fg("dim", "auto-selected after timeout — not a user choice"));
|
||||
}
|
||||
return {
|
||||
header,
|
||||
sections: bodyLines.length > 0 ? [{ lines: bodyLines }] : [],
|
||||
|
||||
@@ -241,15 +241,32 @@ function buildAutoGeneratedError(displayPath: string, detected: string): ToolErr
|
||||
|
||||
const decoder = new TextDecoder("utf-8");
|
||||
|
||||
const autoGeneratedMap = new LRUCache<string, { marker: string | undefined }>({ max: 10 });
|
||||
const autoGeneratedMap = new LRUCache<string, { mtimeMs: number; size: number; marker: string | undefined }>({
|
||||
max: 10,
|
||||
});
|
||||
|
||||
async function getAutoGeneratedMarker(filePath: string): Promise<string | undefined> {
|
||||
if (isAutoGeneratedFileName(filePath)) {
|
||||
return filePath.split("/").pop() ?? "";
|
||||
}
|
||||
|
||||
// Key the cache on (mtime, size) so a file rewritten after the first
|
||||
// check (generator added/removed) is re-scanned instead of served stale.
|
||||
let mtimeMs: number;
|
||||
let size: number;
|
||||
try {
|
||||
const stat = await Bun.file(filePath).stat();
|
||||
mtimeMs = stat.mtimeMs;
|
||||
size = stat.size;
|
||||
} catch (err) {
|
||||
if (isEnoent(err)) {
|
||||
return undefined;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
const cached = autoGeneratedMap.get(filePath);
|
||||
if (cached) return cached.marker;
|
||||
if (cached && cached.mtimeMs === mtimeMs && cached.size === size) return cached.marker;
|
||||
|
||||
let marker: string | undefined;
|
||||
try {
|
||||
@@ -262,7 +279,7 @@ async function getAutoGeneratedMarker(filePath: string): Promise<string | undefi
|
||||
throw err;
|
||||
}
|
||||
|
||||
autoGeneratedMap.set(filePath, { marker });
|
||||
autoGeneratedMap.set(filePath, { mtimeMs, size, marker });
|
||||
return marker;
|
||||
}
|
||||
|
||||
|
||||
@@ -68,7 +68,9 @@ export function scanConflictLines(lines: readonly string[], firstLineNumber: num
|
||||
} | null = null;
|
||||
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
// Strip a trailing \r so CRLF checkouts match the same markers; stored
|
||||
// section lines are LF-normalized (splice re-applies \r on write).
|
||||
const line = stripTrailingCr(lines[i]);
|
||||
const ln = firstLineNumber + i;
|
||||
|
||||
const oursLabel = matchMarker(line, OURS_PREFIX);
|
||||
@@ -338,13 +340,22 @@ export function spliceConflict(originalText: string, entry: ConflictEntry, repla
|
||||
}
|
||||
|
||||
const trimmed = normalizeTrailingNewline(replacement);
|
||||
const replacementLines = trimmed.split("\n");
|
||||
let replacementLines = trimmed.split("\n").map(stripTrailingCr);
|
||||
// Round-trip fidelity for CRLF files: recorded sections are LF-normalized,
|
||||
// so re-apply \r to spliced lines when the matched region used CRLF. The
|
||||
// final replacement line only carries \r when another line follows it.
|
||||
if (lines[match.startIdx]!.endsWith("\r")) {
|
||||
const hasFollowingLine = match.endIdx + 1 < lines.length;
|
||||
replacementLines = replacementLines.map((l, i) =>
|
||||
i < replacementLines.length - 1 || hasFollowingLine ? `${l}\r` : l,
|
||||
);
|
||||
}
|
||||
const next = [...lines.slice(0, match.startIdx), ...replacementLines, ...lines.slice(match.endIdx + 1)];
|
||||
return next.join("\n");
|
||||
}
|
||||
|
||||
/** Reconstruct the recorded marker block as it should appear in the file. */
|
||||
function buildRecordedRegion(entry: ConflictEntry): string[] {
|
||||
function buildRecordedRegion(entry: ConflictBlock): string[] {
|
||||
const out: string[] = [];
|
||||
out.push(entry.oursLabel ? `${OURS_PREFIX} ${entry.oursLabel}` : OURS_PREFIX);
|
||||
out.push(...entry.oursLines);
|
||||
@@ -358,6 +369,36 @@ function buildRecordedRegion(entry: ConflictEntry): string[] {
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when two registered blocks record the same marker-block content
|
||||
* (labels and all sides). Out-of-band edits can shift a block's line
|
||||
* numbers between reads, registering a fresh id while the stale one
|
||||
* persists; callers use content identity to treat a locate-miss for the
|
||||
* stale twin as "already resolved" instead of a hard failure.
|
||||
*/
|
||||
export function conflictRegionsEqual(a: ConflictBlock, b: ConflictBlock): boolean {
|
||||
const ra = buildRecordedRegion(a);
|
||||
const rb = buildRecordedRegion(b);
|
||||
if (ra.length !== rb.length) return false;
|
||||
for (let i = 0; i < ra.length; i++) {
|
||||
if (ra[i] !== rb[i]) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the entry's recorded marker block still occurs in `content`
|
||||
* (LF-normalized — recorded sections are stored LF). Distinguishes a stale
|
||||
* re-registration of a just-resolved region (no longer present) from a
|
||||
* DISTINCT conflict block that happens to be byte-identical (still present
|
||||
* elsewhere in the file and must stay addressable).
|
||||
*/
|
||||
export function conflictRegionPresent(content: string, entry: ConflictBlock): boolean {
|
||||
const region = buildRecordedRegion(entry).join("\n");
|
||||
const normalized = content.includes("\r") ? content.replace(/\r\n/g, "\n") : content;
|
||||
return normalized.includes(region);
|
||||
}
|
||||
|
||||
/**
|
||||
* Find a contiguous match of `expected` inside `lines`, preferring the
|
||||
* occurrence closest to `preferredIdx` to disambiguate when an identical
|
||||
@@ -391,11 +432,16 @@ function locateRegion(
|
||||
function matchesAt(lines: readonly string[], startIdx: number, expected: readonly string[]): boolean {
|
||||
if (startIdx < 0 || startIdx + expected.length > lines.length) return false;
|
||||
for (let i = 0; i < expected.length; i++) {
|
||||
if (lines[startIdx + i] !== expected[i]) return false;
|
||||
// Recorded lines are LF-normalized; tolerate CRLF on-disk lines.
|
||||
if (stripTrailingCr(lines[startIdx + i]!) !== expected[i]) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function stripTrailingCr(line: string): string {
|
||||
return line.endsWith("\r") ? line.slice(0, -1) : line;
|
||||
}
|
||||
|
||||
function normalizeTrailingNewline(replacement: string): string {
|
||||
if (replacement.endsWith("\r\n")) return replacement.slice(0, -2);
|
||||
if (replacement.endsWith("\n")) return replacement.slice(0, -1);
|
||||
|
||||
@@ -244,11 +244,15 @@ function errorResult(text: string, details: IrcDetails): AgentToolResult<IrcDeta
|
||||
return {
|
||||
content: [{ type: "text", text }],
|
||||
details,
|
||||
isError: true,
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeIrcTimeoutMs(value: number): number {
|
||||
if (!Number.isFinite(value) || value === 0) return value === 0 ? 0 : DEFAULT_IRC_TIMEOUT_MS;
|
||||
if (value === 0) return 0; // 0 = timeout disabled
|
||||
// Negative or non-finite settings are misconfigurations — fall back to the
|
||||
// default instead of producing an instant 1 ms timeout.
|
||||
if (!Number.isFinite(value) || value < 0) return DEFAULT_IRC_TIMEOUT_MS;
|
||||
return Math.max(1, Math.trunc(value));
|
||||
}
|
||||
|
||||
|
||||
@@ -285,6 +285,22 @@ function initPhases(entry: TodoOpEntryValue, errors: string[]): TodoPhase[] {
|
||||
errors.push("Missing list for init operation");
|
||||
return [];
|
||||
}
|
||||
// Duplicate phase names / task contents would be permanently unaddressable
|
||||
// (every targeting op resolves the first match), so reject them up front.
|
||||
const seenPhases = new Set<string>();
|
||||
const seenTasks = new Set<string>();
|
||||
for (const listEntry of entry.list) {
|
||||
if (seenPhases.has(listEntry.phase)) {
|
||||
errors.push(`Duplicate phase "${listEntry.phase}" in init list`);
|
||||
}
|
||||
seenPhases.add(listEntry.phase);
|
||||
for (const content of listEntry.items) {
|
||||
if (seenTasks.has(content)) {
|
||||
errors.push(`Duplicate task "${content}" in init list`);
|
||||
}
|
||||
seenTasks.add(content);
|
||||
}
|
||||
}
|
||||
return entry.list.map(listEntry => ({
|
||||
name: listEntry.phase,
|
||||
tasks: listEntry.items.map<TodoItem>(content => ({ content, status: "pending" })),
|
||||
@@ -301,6 +317,19 @@ function appendItems(phases: TodoPhase[], entry: TodoOpEntryValue, errors: strin
|
||||
return phases;
|
||||
}
|
||||
|
||||
// Validate the whole batch before mutating so a failing op reports every
|
||||
// duplicate and leaves nothing half-applied.
|
||||
const seen = new Set<string>();
|
||||
let hasDuplicate = false;
|
||||
for (const content of entry.items) {
|
||||
if (seen.has(content) || findTaskByContent(phases, content)) {
|
||||
errors.push(`Task "${content}" already exists`);
|
||||
hasDuplicate = true;
|
||||
}
|
||||
seen.add(content);
|
||||
}
|
||||
if (hasDuplicate) return phases;
|
||||
|
||||
let phase = findPhaseByName(phases, entry.phase);
|
||||
if (!phase) {
|
||||
phase = { name: entry.phase, tasks: [] };
|
||||
@@ -308,10 +337,6 @@ function appendItems(phases: TodoPhase[], entry: TodoOpEntryValue, errors: strin
|
||||
}
|
||||
|
||||
for (const content of entry.items) {
|
||||
if (findTaskByContent(phases, content)) {
|
||||
errors.push(`Task "${content}" already exists`);
|
||||
return phases;
|
||||
}
|
||||
phase.tasks.push({ content, status: "pending" });
|
||||
}
|
||||
return phases;
|
||||
@@ -618,14 +643,19 @@ export class TodoTool implements AgentTool<typeof todoSchema, TodoToolDetails> {
|
||||
const { phases: updated, errors } = readOnly
|
||||
? { phases: previousPhases, errors: [] as string[] }
|
||||
: applyParams(clonePhases(previousPhases), params);
|
||||
const completedTasks = readOnly ? [] : getCompletionTransitions(previousPhases, updated);
|
||||
if (!readOnly) this.session.setTodoPhases?.(updated);
|
||||
// A batch with any error is discarded wholesale: persisting a
|
||||
// half-applied batch makes the natural retry hit "already exists" for
|
||||
// the ops that did land. State and rendered summary stay at previous.
|
||||
const failed = errors.length > 0;
|
||||
const effective = failed ? previousPhases : updated;
|
||||
const completedTasks = readOnly || failed ? [] : getCompletionTransitions(previousPhases, updated);
|
||||
if (!readOnly && !failed) this.session.setTodoPhases?.(updated);
|
||||
const storage = this.session.getSessionFile() ? "session" : "memory";
|
||||
const details: TodoToolDetails = { phases: updated, storage };
|
||||
const details: TodoToolDetails = { phases: effective, storage };
|
||||
if (completedTasks.length > 0) details.completedTasks = completedTasks;
|
||||
|
||||
return {
|
||||
content: [{ type: "text", text: formatSummary(updated, errors, readOnly) }],
|
||||
content: [{ type: "text", text: formatSummary(effective, errors, readOnly) }],
|
||||
details,
|
||||
isError: errors.length > 0 ? true : undefined,
|
||||
};
|
||||
|
||||
@@ -25,6 +25,8 @@ import { parseArchivePathCandidates } from "./archive-reader";
|
||||
import { assertEditableFile } from "./auto-generated-guard";
|
||||
import {
|
||||
type ConflictEntry,
|
||||
conflictRegionPresent,
|
||||
conflictRegionsEqual,
|
||||
expandContentTokens,
|
||||
getConflictHistory,
|
||||
parseConflictUri,
|
||||
@@ -266,7 +268,14 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
readonly name = "write";
|
||||
readonly approval = (args: unknown) => {
|
||||
const rawPath = (args as Partial<WriteParams>).path;
|
||||
return typeof rawPath === "string" && isInternalUrlPath(rawPath) ? "read" : "write";
|
||||
if (typeof rawPath !== "string" || !isInternalUrlPath(rawPath)) return "write";
|
||||
// Internal URLs are usually session-local artifacts (read tier), but a
|
||||
// scheme whose handler exposes a `write` hook mutates handler-owned
|
||||
// user data (e.g. vault:// notes, host-owned mcp:// URIs) and must take
|
||||
// the write tier so always-ask mode actually prompts.
|
||||
const match = /^([a-z][a-z0-9+.-]*):\/\//i.exec(rawPath.trim());
|
||||
const handler = match ? InternalUrlRouter.instance().getHandler(match[1]!.toLowerCase()) : undefined;
|
||||
return handler?.write ? "write" : "read";
|
||||
};
|
||||
readonly formatApprovalDetails = (args: unknown): string[] => {
|
||||
const params = args as Partial<WriteParams>;
|
||||
@@ -349,7 +358,18 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
content: string,
|
||||
resolvedArchivePath: ResolvedArchiveWritePath,
|
||||
): Promise<AgentToolResult<WriteToolDetails>> {
|
||||
const isZip = resolvedArchivePath.absolutePath.toLowerCase().endsWith(".zip");
|
||||
// Resolve symlinks before the tmp+rename swap: renaming over a symlink
|
||||
// replaces the link itself with a regular file instead of writing
|
||||
// through to its target.
|
||||
const finalPath = resolvedArchivePath.exists
|
||||
? await fs.realpath(resolvedArchivePath.absolutePath).catch(() => resolvedArchivePath.absolutePath)
|
||||
: resolvedArchivePath.absolutePath;
|
||||
const lowerPath = finalPath.toLowerCase();
|
||||
const isZip = lowerPath.endsWith(".zip");
|
||||
const isGzip = lowerPath.endsWith(".tar.gz") || lowerPath.endsWith(".tgz");
|
||||
// Rewrites are whole-archive: write to a temp file and rename so a
|
||||
// crash/disk-full mid-write can't destroy the original archive.
|
||||
const tmpPath = `${finalPath}.tmp-${process.pid}`;
|
||||
|
||||
const parentDir = path.dirname(resolvedArchivePath.absolutePath);
|
||||
if (parentDir && parentDir !== ".") {
|
||||
@@ -377,8 +397,10 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
try {
|
||||
const { zipSync } = await loadFflate();
|
||||
const zipBuffer = zipSync(zipEntries);
|
||||
await Bun.write(resolvedArchivePath.absolutePath, zipBuffer);
|
||||
await Bun.write(tmpPath, zipBuffer);
|
||||
await fs.rename(tmpPath, finalPath);
|
||||
} catch (error) {
|
||||
await fs.rm(tmpPath, { force: true }).catch(() => {});
|
||||
throw new ToolError(error instanceof Error ? error.message : String(error));
|
||||
}
|
||||
} else {
|
||||
@@ -406,8 +428,12 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
archiveEntries[resolvedArchivePath.archiveSubPath] = content;
|
||||
|
||||
try {
|
||||
await Bun.Archive.write(resolvedArchivePath.absolutePath, archiveEntries);
|
||||
// `Bun.Archive.write` never infers compression from the extension;
|
||||
// request gzip explicitly so `.tar.gz`/`.tgz` stay compressed.
|
||||
await Bun.Archive.write(tmpPath, archiveEntries, isGzip ? { compress: "gzip" } : undefined);
|
||||
await fs.rename(tmpPath, finalPath);
|
||||
} catch (error) {
|
||||
await fs.rm(tmpPath, { force: true }).catch(() => {});
|
||||
throw new ToolError(error instanceof Error ? error.message : String(error));
|
||||
}
|
||||
}
|
||||
@@ -583,7 +609,24 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
invalidateFsScanAfterWrite(absolutePath);
|
||||
this.session.bumpFileMutationVersion?.(absolutePath);
|
||||
this.session.fileSnapshotStore?.invalidate(absolutePath);
|
||||
this.session.conflictHistory?.invalidate(entry.id);
|
||||
const history = this.session.conflictHistory;
|
||||
history?.invalidate(entry.id);
|
||||
if (history) {
|
||||
// Drop stale duplicate registrations of the same region: a re-read
|
||||
// after an out-of-band shift registers a fresh id at the new
|
||||
// startLine while the stale twin persists at the old one. A DISTINCT
|
||||
// conflict block that is merely byte-identical still occurs in the
|
||||
// post-splice content and must stay addressable.
|
||||
for (const other of history.entries()) {
|
||||
if (
|
||||
other.absolutePath === absolutePath &&
|
||||
conflictRegionsEqual(other, entry) &&
|
||||
!conflictRegionPresent(newContent, other)
|
||||
) {
|
||||
history.invalidate(other.id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const header = maybeWriteSnapshotHeader(this.session, absolutePath, newContent);
|
||||
const range =
|
||||
@@ -690,12 +733,11 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
fileEntries.sort((a, b) => b.startLine - a.startLine);
|
||||
|
||||
let text: string;
|
||||
const resolvedEntries: ConflictEntry[] = [];
|
||||
const staleEntries: ConflictEntry[] = [];
|
||||
let failure: string | undefined;
|
||||
try {
|
||||
text = await Bun.file(absolutePath).text();
|
||||
for (const entry of fileEntries) {
|
||||
const expanded = expandContentTokens(replacementContent, entry);
|
||||
text = spliceConflict(text, entry, expanded);
|
||||
}
|
||||
} catch (error) {
|
||||
failedFiles.push({
|
||||
displayPath: sample.displayPath,
|
||||
@@ -704,15 +746,41 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
});
|
||||
continue;
|
||||
}
|
||||
for (const entry of fileEntries) {
|
||||
try {
|
||||
const expanded = expandContentTokens(replacementContent, entry);
|
||||
text = spliceConflict(text, entry, expanded);
|
||||
resolvedEntries.push(entry);
|
||||
} catch (error) {
|
||||
// A locate-miss for a region an earlier entry already spliced
|
||||
// in this pass is a stale duplicate registration (re-read after
|
||||
// an out-of-band shift) — treat it as already resolved.
|
||||
if (resolvedEntries.some(done => conflictRegionsEqual(done, entry))) {
|
||||
staleEntries.push(entry);
|
||||
continue;
|
||||
}
|
||||
failure = error instanceof Error ? error.message : String(error);
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (failure !== undefined) {
|
||||
failedFiles.push({
|
||||
displayPath: sample.displayPath,
|
||||
count: fileEntries.length,
|
||||
error: failure,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
const diagnostics = await this.#writethrough(absolutePath, text, signal, undefined, batchRequest);
|
||||
invalidateFsScanAfterWrite(absolutePath);
|
||||
this.session.bumpFileMutationVersion?.(absolutePath);
|
||||
this.session.fileSnapshotStore?.invalidate(absolutePath);
|
||||
for (const entry of fileEntries) history.invalidate(entry.id);
|
||||
for (const entry of resolvedEntries) history.invalidate(entry.id);
|
||||
for (const entry of staleEntries) history.invalidate(entry.id);
|
||||
const header = maybeWriteSnapshotHeader(this.session, absolutePath, text);
|
||||
succeededFiles.push({ displayPath: sample.displayPath, count: fileEntries.length, header });
|
||||
totalResolvedIds += fileEntries.length;
|
||||
succeededFiles.push({ displayPath: sample.displayPath, count: resolvedEntries.length, header });
|
||||
totalResolvedIds += resolvedEntries.length;
|
||||
if (diagnostics) allDiagnostics.push(diagnostics);
|
||||
}
|
||||
|
||||
@@ -751,7 +819,11 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
if (failedFiles.length > 0 && succeededFiles.length === 0) {
|
||||
throw new ToolError(resultText);
|
||||
}
|
||||
return { content: [{ type: "text", text: resultText }], details: {} };
|
||||
return {
|
||||
content: [{ type: "text", text: resultText }],
|
||||
details: {},
|
||||
isError: failedFiles.length > 0 ? true : undefined,
|
||||
};
|
||||
}
|
||||
const mergedSummary = allDiagnostics.map(d => d.summary).join("\n");
|
||||
const mergedMessages = allDiagnostics.flatMap(d => d.messages ?? []);
|
||||
@@ -760,6 +832,7 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
details: {
|
||||
meta: outputMeta().diagnostics(mergedSummary, mergedMessages).get(),
|
||||
},
|
||||
isError: failedFiles.length > 0 ? true : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -784,6 +857,9 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
const scheme = parsed.protocol.replace(/:$/, "").toLowerCase();
|
||||
const handler = internalRouter.getHandler(scheme);
|
||||
if (handler?.write) {
|
||||
// Handler-owned writes (vault:// notes, host URIs) mutate user
|
||||
// data outside the local sandbox — plan mode must reject them.
|
||||
enforcePlanModeWrite(this.session, path, { op: "update" });
|
||||
await handler.write(parsed, cleanContent, { cwd: this.session.cwd, signal });
|
||||
let resultText = `Successfully wrote ${cleanContent.length} bytes to ${path}`;
|
||||
if (stripped) {
|
||||
@@ -872,6 +948,8 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
throw new ToolError(error instanceof Error ? error.message : String(error));
|
||||
}
|
||||
invalidateFsScanAfterWrite(absolutePath);
|
||||
this.session.bumpFileMutationVersion?.(absolutePath);
|
||||
const madeExecutable = await maybeMarkExecutableForShebang(absolutePath, cleanContent);
|
||||
const displayPath = formatPathRelativeToCwd(absolutePath, this.session.cwd);
|
||||
const header = maybeWriteSnapshotHeader(this.session, absolutePath, cleanContent);
|
||||
const writeLine = `Successfully wrote ${cleanContent.length} bytes to ${displayPath}`;
|
||||
@@ -881,7 +959,7 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
|
||||
}
|
||||
return {
|
||||
content: [{ type: "text", text: resultText }],
|
||||
details: { resolvedPath: absolutePath },
|
||||
details: { resolvedPath: absolutePath, madeExecutable: madeExecutable || undefined },
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -567,6 +567,23 @@ describe("Coding Agent Tools", () => {
|
||||
expect(output).toContain("Use :1 to read from the start, or :3 to read the last line.");
|
||||
});
|
||||
|
||||
it("should emit a binary notice instead of mojibake for files with NUL bytes", async () => {
|
||||
const testFile = path.join(testDir, "blob.bin");
|
||||
fs.writeFileSync(testFile, Buffer.from([0x61, 0x62, 0x63, 0x00, 0xff, 0xfe, 0x64, 0x65]));
|
||||
|
||||
const result = await readTool.execute("test-call-binary-nul", { path: testFile });
|
||||
const output = getTextOutput(result);
|
||||
|
||||
expect(output).toContain("Cannot read binary file");
|
||||
expect(output).toContain("NUL bytes");
|
||||
});
|
||||
|
||||
it("should reject malformed internal-URL selectors instead of dumping the whole resource", async () => {
|
||||
await expect(readTool.execute("test-call-bad-internal-sel", { path: "artifact://3:-100" })).rejects.toThrow(
|
||||
/Invalid selector ':-100'/,
|
||||
);
|
||||
});
|
||||
|
||||
it("should include truncation details when truncated", async () => {
|
||||
const testFile = path.join(testDir, "large-file.txt");
|
||||
const lines = Array.from({ length: 3500 }, (_, i) => `Line ${i + 1}`);
|
||||
@@ -719,6 +736,53 @@ describe("Coding Agent Tools", () => {
|
||||
});
|
||||
}
|
||||
|
||||
it("should treat a selector-shaped archive subpath as a root listing selector", async () => {
|
||||
const archivePath = path.join(testDir, "root-selector.tar");
|
||||
fs.writeFileSync(
|
||||
archivePath,
|
||||
createTarArchive([
|
||||
{ path: "alpha.txt", content: "alpha\n" },
|
||||
{ path: "beta.txt", content: "beta\n" },
|
||||
]),
|
||||
);
|
||||
|
||||
// Previously misparsed as a member named "2" and failed with a
|
||||
// misleading "not found inside archive" error. The selector is honored
|
||||
// as a 1-indexed listing offset, so `:2` starts at the second entry.
|
||||
const result = await readTool.execute("test-call-archive-root-selector", { path: `${archivePath}:2` });
|
||||
const output = getTextOutput(result);
|
||||
|
||||
expect(output).toContain("beta.txt");
|
||||
expect(output).not.toContain("alpha.txt");
|
||||
expect(result.details?.isDirectory).toBe(true);
|
||||
});
|
||||
|
||||
it("should prefer an archive member over a selector-shaped name", async () => {
|
||||
const archivePath = path.join(testDir, "member-precedence.tar");
|
||||
fs.writeFileSync(archivePath, createTarArchive([{ path: "raw", content: "member named raw\n" }]));
|
||||
|
||||
const result = await readTool.execute("test-call-archive-member-raw", { path: `${archivePath}:raw` });
|
||||
const output = getTextOutput(result);
|
||||
|
||||
expect(output).toContain("member named raw");
|
||||
});
|
||||
|
||||
it("should reject archive members larger than the in-memory extraction cap", async () => {
|
||||
const archivePath = path.join(testDir, "bomb.zip");
|
||||
fs.writeFileSync(
|
||||
archivePath,
|
||||
createZipArchiveWithRawDeflateEntry({
|
||||
path: "bomb.bin",
|
||||
compressed: Buffer.from([0xff, 0xff, 0xff, 0xff]),
|
||||
originalSize: 3 * 1024 * 1024 * 1024, // 3GB declared, never allocated
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(readTool.execute("test-call-archive-bomb", { path: `${archivePath}:bomb.bin` })).rejects.toThrow(
|
||||
/too large to extract/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("should detect image MIME type from file magic (not extension)", async () => {
|
||||
const png1x1Base64 =
|
||||
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+X2Z0AAAAASUVORK5CYII=";
|
||||
@@ -870,6 +934,36 @@ describe("Coding Agent Tools", () => {
|
||||
expect(await files.get("pkg/new.txt")?.text()).toBe(content);
|
||||
});
|
||||
|
||||
it("should preserve gzip compression when writing into an existing .tar.gz", async () => {
|
||||
const archivePath = path.join(testDir, "write-existing.tar.gz");
|
||||
fs.writeFileSync(
|
||||
archivePath,
|
||||
zlib.gzipSync(
|
||||
createTarArchive([
|
||||
{ path: "pkg/README.md", content: "# Original\n" },
|
||||
{ path: "pkg/src/index.ts", content: "export const archiveValue = 1;\n" },
|
||||
]),
|
||||
),
|
||||
);
|
||||
|
||||
const content = "# Updated\nLine 2\n";
|
||||
await writeTool.execute("test-call-archive-write-targz", {
|
||||
path: `${archivePath}:pkg/README.md`,
|
||||
content,
|
||||
});
|
||||
|
||||
const bytes = fs.readFileSync(archivePath);
|
||||
// gzip magic must survive the rewrite (regression: archive was
|
||||
// silently rewritten as a bare tar under the .gz name).
|
||||
expect(bytes[0]).toBe(0x1f);
|
||||
expect(bytes[1]).toBe(0x8b);
|
||||
|
||||
const archive = new Bun.Archive(await Bun.file(archivePath).bytes());
|
||||
const files = await archive.files();
|
||||
expect(await files.get("pkg/README.md")?.text()).toBe(content);
|
||||
expect(await files.get("pkg/src/index.ts")?.text()).toBe("export const archiveValue = 1;\n");
|
||||
});
|
||||
|
||||
it("should treat a plain archive filename as a regular file write", async () => {
|
||||
const archivePath = path.join(testDir, "literal.zip");
|
||||
const content = "plain file contents\n";
|
||||
|
||||
@@ -94,6 +94,15 @@ describe("scanConflictLines", () => {
|
||||
expect(blocks[0].oursLabel).toBe("second");
|
||||
expect(blocks[0].oursLines).toEqual(["good ours"]);
|
||||
});
|
||||
|
||||
it("detects conflicts in CRLF files and stores LF-normalized sections", () => {
|
||||
const blocks = scanConflictLines(["<<<<<<< HEAD\r", "ours\r", "=======\r", "theirs\r", ">>>>>>> feat\r"], 1);
|
||||
expect(blocks).toHaveLength(1);
|
||||
expect(blocks[0].oursLabel).toBe("HEAD");
|
||||
expect(blocks[0].theirsLabel).toBe("feat");
|
||||
expect(blocks[0].oursLines).toEqual(["ours"]);
|
||||
expect(blocks[0].theirsLines).toEqual(["theirs"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ConflictHistory", () => {
|
||||
@@ -291,6 +300,20 @@ describe("spliceConflict", () => {
|
||||
it("rejects when the file is shorter than the recorded region", () => {
|
||||
expect(() => spliceConflict("short\n", entry, "x\n")).toThrow(/no longer present/);
|
||||
});
|
||||
|
||||
it("splices CRLF files and preserves CRLF line endings", () => {
|
||||
const crlfFile = ["before", "<<<<<<< HEAD", "ours", "=======", "theirs", ">>>>>>> feat", "after", ""].join(
|
||||
"\r\n",
|
||||
);
|
||||
const result = spliceConflict(crlfFile, entry, "alpha\nbeta\n");
|
||||
expect(result).toBe("before\r\nalpha\r\nbeta\r\nafter\r\n");
|
||||
});
|
||||
|
||||
it("does not append \\r when the spliced region ends the file without a trailing newline", () => {
|
||||
const crlfNoEof = ["before", "<<<<<<< HEAD", "ours", "=======", "theirs", ">>>>>>> feat"].join("\r\n");
|
||||
const result = spliceConflict(crlfNoEof, entry, "resolved");
|
||||
expect(result).toBe("before\r\nresolved");
|
||||
});
|
||||
});
|
||||
|
||||
describe("renderConflictRegion", () => {
|
||||
|
||||
Reference in New Issue
Block a user