feat(coding-agent): implemented pattern-based bash interceptor rules + exclusive range semantics
- Changed bash interceptor configuration from boolean flags to customizable pattern-based rules array. - Clarified hashline range replace semantics: end parameter is now strictly exclusive boundary. - Fixed bash interceptor to apply built-in default rules when no custom patterns are configured. - Updated hashline range validation and calculations to enforce exclusive end semantics throughout.
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Added ACP (Agent Client Protocol) mode for headless agent operation via `--mode acp`
|
||||
@@ -10,11 +9,17 @@
|
||||
|
||||
### Changed
|
||||
|
||||
- Updated bash interceptor configuration to use customizable pattern rules instead of individual boolean flags
|
||||
- Clarified hashline range replace semantics: `end` parameter is now strictly exclusive (the line it points to survives and is not consumed)
|
||||
- Updated ask tool rendering to support markdown formatting in questions and option labels
|
||||
- Refactored hook input and selector components to render titles as markdown for richer text formatting
|
||||
- Changed session collection to include sessions with zero messages, enabling ACP mode to create discoverable sessions immediately
|
||||
- Changed session persistence logic to use atomic file rewrite when flushing unflushed sessions to prevent duplication
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed bash interceptor to apply built-in default rules when no custom patterns are configured
|
||||
|
||||
## [13.14.0] - 2026-03-20
|
||||
|
||||
### Added
|
||||
|
||||
@@ -139,6 +139,43 @@ type SettingDef =
|
||||
// under `as const` while still letting SettingValue infer the correct element type.
|
||||
const EMPTY_STRING_ARRAY: string[] = [];
|
||||
const EMPTY_STRING_RECORD: Record<string, string> = {};
|
||||
export const DEFAULT_BASH_INTERCEPTOR_RULES: BashInterceptorRule[] = [
|
||||
{
|
||||
pattern: "^\\s*(cat|head|tail|less|more)\\s+",
|
||||
tool: "read",
|
||||
message: "Use the `read` tool instead of cat/head/tail. It provides better context and handles binary files.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*(grep|rg|ripgrep|ag|ack)\\s+",
|
||||
tool: "grep",
|
||||
message: "Use the `grep` tool instead of grep/rg. It respects .gitignore and provides structured output.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*(find|fd|locate)\\s+.*(-name|-iname|-type|--type|-glob)",
|
||||
tool: "find",
|
||||
message: "Use the `find` tool instead of find/fd. It respects .gitignore and is faster for glob patterns.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*sed\\s+(-i|--in-place)",
|
||||
tool: "edit",
|
||||
message: "Use the `edit` tool instead of sed -i. It provides diff preview and fuzzy matching.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*perl\\s+.*-[pn]?i",
|
||||
tool: "edit",
|
||||
message: "Use the `edit` tool instead of perl -i. It provides diff preview and fuzzy matching.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*awk\\s+.*-i\\s+inplace",
|
||||
tool: "edit",
|
||||
message: "Use the `edit` tool instead of awk -i inplace. It provides diff preview and fuzzy matching.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*(echo|printf|cat\\s*<<)\\s+.*[^|]>\\s*\\S",
|
||||
tool: "write",
|
||||
message: "Use the `write` tool instead of echo/cat redirection. It handles encoding and provides confirmation.",
|
||||
},
|
||||
];
|
||||
|
||||
export const SETTINGS_SCHEMA = {
|
||||
// ────────────────────────────────────────────────────────────────────────
|
||||
@@ -943,16 +980,7 @@ export const SETTINGS_SCHEMA = {
|
||||
default: false,
|
||||
ui: { tab: "editing", label: "Bash Interceptor", description: "Block shell commands that have dedicated tools" },
|
||||
},
|
||||
|
||||
"bashInterceptor.simpleLs": {
|
||||
type: "boolean",
|
||||
default: true,
|
||||
ui: {
|
||||
tab: "editing",
|
||||
label: "Intercept `ls`",
|
||||
description: "Intercept bare ls commands (when interceptor is enabled)",
|
||||
},
|
||||
},
|
||||
"bashInterceptor.patterns": { type: "array", default: DEFAULT_BASH_INTERCEPTOR_RULES },
|
||||
|
||||
// Python
|
||||
"python.toolMode": {
|
||||
|
||||
@@ -341,10 +341,7 @@ export class Settings {
|
||||
* Get bash interceptor rules (typed accessor for complex array config).
|
||||
*/
|
||||
getBashInterceptorRules(): BashInterceptorRule[] {
|
||||
const patterns = (this.#merged.bashInterceptor as { patterns?: unknown[] })?.patterns;
|
||||
if (!Array.isArray(patterns)) return [];
|
||||
|
||||
return patterns.filter((p): p is BashInterceptorRule => typeof p === "object" && p !== null && "pattern" in p);
|
||||
return this.get("bashInterceptor.patterns");
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -15,6 +15,13 @@
|
||||
import type { HashMismatch } from "./types";
|
||||
|
||||
export type Anchor = { line: number; hash: string };
|
||||
/**
|
||||
* Edit operation on hashline-addressed content.
|
||||
*
|
||||
* For range replace: `pos` is inclusive (first consumed line),
|
||||
* `end` is **exclusive** (first surviving line after the range).
|
||||
* The consumed range is `[pos.line, end.line - 1]`.
|
||||
*/
|
||||
export type HashlineEdit =
|
||||
| { op: "replace"; pos: Anchor; end?: Anchor; lines: string[] }
|
||||
| { op: "append"; pos?: Anchor; lines: string[] }
|
||||
@@ -470,9 +477,8 @@ function shouldAutocorrect(line: string, otherLine: string): boolean {
|
||||
/**
|
||||
* Apply an array of hashline edits to file content.
|
||||
*
|
||||
* Each edit operation identifies target lines directly (`replace`,
|
||||
* `append`, `prepend`). Line references are resolved via {@link parseTag}
|
||||
* and hashes validated before any mutation.
|
||||
* For range replace, `end` is **exclusive**: the consumed range is
|
||||
* `[pos.line, end.line - 1]` and the line at `end` survives.
|
||||
*
|
||||
* Edits are sorted bottom-up (highest effective line first) so earlier
|
||||
* splices don't invalidate later line numbers.
|
||||
@@ -518,8 +524,10 @@ export function applyHashlineEdits(
|
||||
const startValid = validateRef(edit.pos);
|
||||
const endValid = validateRef(edit.end);
|
||||
if (!startValid || !endValid) continue;
|
||||
if (edit.pos.line > edit.end.line) {
|
||||
throw new Error(`Range start line ${edit.pos.line} must be <= end line ${edit.end.line}`);
|
||||
if (edit.pos.line >= edit.end.line) {
|
||||
throw new Error(
|
||||
`Range start line ${edit.pos.line} must be < end line ${edit.end.line} (end is exclusive)`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
if (!validateRef(edit.pos)) continue;
|
||||
@@ -597,10 +605,13 @@ export function applyHashlineEdits(
|
||||
case "replace":
|
||||
if (!edit.end) {
|
||||
sortLine = edit.pos.line;
|
||||
precedence = 0;
|
||||
} else {
|
||||
sortLine = edit.end.line;
|
||||
// Range replaces must run after edits anchored on the surviving end line,
|
||||
// so those line-number references still point at the same survivor.
|
||||
precedence = 3;
|
||||
}
|
||||
precedence = 0;
|
||||
break;
|
||||
case "append":
|
||||
sortLine = edit.pos ? edit.pos.line : fileLines.length + 1;
|
||||
@@ -634,19 +645,22 @@ export function applyHashlineEdits(
|
||||
fileLines.splice(edit.pos.line - 1, 1, ...newLines);
|
||||
trackFirstChanged(edit.pos.line);
|
||||
} else {
|
||||
const count = edit.end.line - edit.pos.line + 1;
|
||||
// end is exclusive: consumed range is [pos.line, end.line - 1]
|
||||
const count = edit.end.line - edit.pos.line;
|
||||
const newLines = [...edit.lines];
|
||||
// The end line itself survives (exclusive). If the model re-emits it
|
||||
// in lines, that's a duplication mistake — auto-correct by popping.
|
||||
const trailingReplacementLine = newLines[newLines.length - 1]?.trimEnd();
|
||||
const nextSurvivingLine = fileLines[edit.end.line]?.trimEnd();
|
||||
const nextSurvivingLine = fileLines[edit.end.line - 1]?.trimEnd();
|
||||
if (
|
||||
shouldAutocorrect(trailingReplacementLine, nextSurvivingLine) &&
|
||||
// Safety: only correct when end-line content differs from the duplicate.
|
||||
// If end already points to the boundary, matching next line is coincidence.
|
||||
fileLines[edit.end.line - 1]?.trimEnd() !== trailingReplacementLine
|
||||
// Safety: only correct when the last consumed line differs from the duplicate.
|
||||
// If the last consumed line is the same as the surviving end line, it's coincidence.
|
||||
fileLines[edit.end.line - 2]?.trimEnd() !== trailingReplacementLine
|
||||
) {
|
||||
newLines.pop();
|
||||
warnings.push(
|
||||
`Auto-corrected range replace ${edit.pos.line}#${edit.pos.hash}-${edit.end.line}#${edit.end.hash}: removed trailing replacement line "${trailingReplacementLine}" that duplicated next surviving line`,
|
||||
`Auto-corrected range replace ${edit.pos.line}#${edit.pos.hash}..${edit.end.line}#${edit.end.hash}: removed trailing replacement line "${trailingReplacementLine}" that duplicated the surviving end line`,
|
||||
);
|
||||
}
|
||||
const leadingReplacementLine = newLines[0]?.trimEnd();
|
||||
@@ -659,7 +673,7 @@ export function applyHashlineEdits(
|
||||
) {
|
||||
newLines.shift();
|
||||
warnings.push(
|
||||
`Auto-corrected range replace ${edit.pos.line}#${edit.pos.hash}-${edit.end.line}#${edit.end.hash}: removed leading replacement line "${leadingReplacementLine}" that duplicated preceding surviving line`,
|
||||
`Auto-corrected range replace ${edit.pos.line}#${edit.pos.hash}..${edit.end.line}#${edit.end.hash}: removed leading replacement line "${leadingReplacementLine}" that duplicated preceding surviving line`,
|
||||
);
|
||||
}
|
||||
fileLines.splice(edit.pos.line - 1, count, ...newLines);
|
||||
|
||||
@@ -11,14 +11,13 @@ Read the file first to get fresh tags. Submit one `edit` call per file with all
|
||||
- if `replace`: first line to rewrite
|
||||
- if `prepend`: line to insert new lines **before**; omit for beginning of file
|
||||
- if `append`: line to insert new lines **after**; omit for end of file
|
||||
**`edits[n].end`** — range replace only. The last line of the range (inclusive). Omit for single-line replace.
|
||||
**`edits[n].end`** — range replace only. The first line **after** the range (exclusive — this line survives). Omit for single-line replace.
|
||||
**`edits[n].lines`** — the replacement content:
|
||||
- for `replace`: the exact lines that will replace `[pos, end??pos]` inclusively (or the single `pos` line when `end` is omitted)
|
||||
- for `replace`: the lines that will replace `[pos, end)`. Everything from `pos` up to (but not including) `end` is removed; `lines` is inserted in its place.
|
||||
- for `prepend`/`append`: the new lines to insert
|
||||
- `[""]` — blank line
|
||||
- `null` or `[]` — delete if replace
|
||||
- If `lines` contains content that already exists after `end`, those lines **will be duplicated** in the output.
|
||||
- Keep `lines` to exactly what belongs inside the consumed range.
|
||||
- **`end` is exclusive — the line it points to stays in the file.** You do not need to re-emit it in `lines`. If you accidentally include it in `lines`, it will be duplicated.
|
||||
- Ops are applied bottom-up. Tags **MUST** be referenced from the most recent `read` output.
|
||||
</operations>
|
||||
|
||||
@@ -71,22 +70,22 @@ Single line — `lines: null` deletes entirely:
|
||||
}]
|
||||
}
|
||||
```
|
||||
Range — remove the legacy block (lines 10–11):
|
||||
Range — remove the legacy block (lines 10–11). `end` points to line 12 (the line after the range):
|
||||
```
|
||||
{
|
||||
path: "util.ts",
|
||||
edits: [{
|
||||
op: "replace",
|
||||
pos: {{hlineref 10 "\t// TODO: remove after migration"}},
|
||||
end: {{hlineref 11 "\tlegacy();"}},
|
||||
end: {{hlineref 12 "\ttry {"}},
|
||||
lines: null
|
||||
}]
|
||||
}
|
||||
```
|
||||
</example>
|
||||
|
||||
<example name="rewrite a block body — shape (a)">
|
||||
Replace the catch body with smarter error handling. Shape (a): `pos` is the first body line, `end` is the last body line. The catch header (line 14) and its closer (line 17) are outside the range and stay untouched.
|
||||
<example name="rewrite a block body">
|
||||
Replace the catch body with smarter error handling. `pos` is the first body line, `end` is the closer — the closer survives automatically.
|
||||
|
||||
When changing body content, replace the **entire** body span — not just one line inside it. Patching one line leaves the rest of the body stale.
|
||||
```
|
||||
@@ -95,7 +94,7 @@ When changing body content, replace the **entire** body span — not just one li
|
||||
edits: [{
|
||||
op: "replace",
|
||||
pos: {{hlineref 15 "\t\tconsole.error(err);"}},
|
||||
end: {{hlineref 16 "\t\treturn null;"}},
|
||||
end: {{hlineref 17 "\t}"}},
|
||||
lines: [
|
||||
"\t\tif (isEnoent(err)) return null;",
|
||||
"\t\tthrow err;"
|
||||
@@ -103,28 +102,15 @@ When changing body content, replace the **entire** body span — not just one li
|
||||
}]
|
||||
}
|
||||
```
|
||||
Result: lines 15–16 are replaced. The `\t}` on line 17 stays because `end` is exclusive.
|
||||
</example>
|
||||
|
||||
<example name="replace whole block — shape (b)">
|
||||
Simplify `beta()` to a one-liner. Shape (b): `pos`=header, `end`=closer, re-emit all in `lines`.
|
||||
<example name="replace whole block">
|
||||
Simplify `beta()` to a one-liner. `pos`=header (consumed), `end`=the line **after** the block (survives).
|
||||
|
||||
Bad — `end` stops at the inner `\t}` on line 17, so the outer `}` on line 18 survives. Result: two consecutive `}` lines.
|
||||
```
|
||||
{
|
||||
path: "util.ts",
|
||||
edits: [{
|
||||
op: "replace",
|
||||
pos: {{hlineref 9 "function beta() {"}},
|
||||
end: {{hlineref 17 "\t}"}},
|
||||
lines: [
|
||||
"function beta() {",
|
||||
"\treturn parse(data);",
|
||||
"}"
|
||||
]
|
||||
}]
|
||||
}
|
||||
```
|
||||
Good — `end` includes the function's own `}` on line 18, so the old closer is consumed:
|
||||
Since `}` on line 18 is the last line of `beta()` and we want to consume it, `end` must point to the next line after the block. When line 18 is the last line of the file, omit `end` — single-line replace plus a delete of lines 10–17 first, or use `write` to rewrite the file.
|
||||
|
||||
When there IS a line after the block:
|
||||
```
|
||||
{
|
||||
path: "util.ts",
|
||||
@@ -134,12 +120,12 @@ Good — `end` includes the function's own `}` on line 18, so the old closer is
|
||||
end: {{hlineref 18 "}"}},
|
||||
lines: [
|
||||
"function beta() {",
|
||||
"\treturn parse(data);",
|
||||
"}"
|
||||
"\treturn parse(data);"
|
||||
]
|
||||
}]
|
||||
}
|
||||
```
|
||||
Result: lines 9–17 are consumed (replaced). `}` on line 18 survives as beta's closer — no need to re-emit it.
|
||||
</example>
|
||||
|
||||
<example name="avoid shared boundary lines">
|
||||
@@ -149,7 +135,7 @@ Bad — if you need to change code on both sides of that line, replacing just th
|
||||
|
||||
Good — choose one of two safe shapes instead:
|
||||
- move inward and replace only body-owned lines
|
||||
- expand outward and replace one whole owned block, consuming its real closer/separator too
|
||||
- expand outward and replace one whole owned block
|
||||
</example>
|
||||
|
||||
<example name="insert between sibling declarations">
|
||||
@@ -180,9 +166,9 @@ Use a trailing `""` to preserve the blank line between sibling declarations.
|
||||
- For `append`/`prepend`, `lines` **MUST** contain only the newly introduced content. Do not re-emit surrounding content, or terminators that already exist.
|
||||
- When changing existing code near a block tail or closing delimiter, default to `replace` over the owned span instead of inserting around the boundary.
|
||||
- When adding a sibling declaration, default to `prepend` on the next sibling declaration instead of `append` on the previous block's closing brace.
|
||||
- **Block boundaries travel together.** For a block `{ header / body / closer }`, there are exactly two valid replace shapes: (a) replace only the body — `pos`=first body line, `end`=last body line, leave the header and closer untouched; or (b) replace the whole block — `pos`=header, `end`=closer, re-emit all three in `lines`. Never split them: do not set `end` to the closer while omitting it from `lines` (deletes it), and do not emit the closer in `lines` without including it in `end` (duplicates it). This applies to every block terminator: `}`, `continue`, `break`, `return`, `throw`.
|
||||
- **Never target shared boundary lines.** Do not use `replace` spans that start, end, or pivot on a line that closes one construct and opens/separates another, such as `},{`, `}),`, `} else {`, or `} catch (err) {`. Those lines are not owned by a single block. Move the range inward to body-only lines, or widen it to consume one whole owned construct including its true trailing delimiter.
|
||||
- **`lines` must not extend past `end`.** `lines` replaces exactly `pos..end`. Content after `end` survives. If you include lines in `lines` that exist after `end`, they will appear twice. Either extend `end` to cover all lines you are re-emitting, or remove the extra lines from `lines`.
|
||||
- **`end` is the boundary you want to keep.** Point `end` at the closing delimiter (`}`, `)`, `</tag>`) when you want it to survive. Point `end` past it when you want to consume it. Do **not** include the `end` line in `lines` — it survives on its own.
|
||||
- **Never target shared boundary lines.** Do not use `replace` spans that start, end, or pivot on a line that closes one construct and opens/separates another, such as `},{`, `}),`, `} else {`, or `} catch (err) {`. Those lines are not owned by a single block. Move the range inward to body-only lines, or widen it to consume one whole owned construct.
|
||||
- **`lines` must not extend past `end`.** `lines` replaces exactly `[pos, end)`. The `end` line and everything after it survives. If you include `end`-line content in `lines`, it will appear twice.
|
||||
- `lines` entries **MUST** be literal file content with indentation copied exactly from the `read` output. If the file uses tabs, use a real tab character.
|
||||
- After any successful `edit` call on a file, the next change to that same file **MUST** start with a fresh `read`. Do not chain a second `edit` call off stale mental state, even if the intended range is nearby.
|
||||
- If you need a second change in the same local region, default to one wider `replace` over the whole owned block instead of a sequence of micro-edits on adjacent lines. Repeated small patches in a moving region are unstable.
|
||||
|
||||
@@ -5,45 +5,7 @@
|
||||
* this interceptor provides helpful error messages directing them to use
|
||||
* the specialized tools instead.
|
||||
*/
|
||||
import type { BashInterceptorRule } from "../config/settings-schema";
|
||||
|
||||
export const DEFAULT_BASH_INTERCEPTOR_RULES: BashInterceptorRule[] = [
|
||||
{
|
||||
pattern: "^\\s*(cat|head|tail|less|more)\\s+",
|
||||
tool: "read",
|
||||
message: "Use the `read` tool instead of cat/head/tail. It provides better context and handles binary files.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*(grep|rg|ripgrep|ag|ack)\\s+",
|
||||
tool: "grep",
|
||||
message: "Use the `grep` tool instead of grep/rg. It respects .gitignore and provides structured output.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*(find|fd|locate)\\s+.*(-name|-iname|-type|--type|-glob)",
|
||||
tool: "find",
|
||||
message: "Use the `find` tool instead of find/fd. It respects .gitignore and is faster for glob patterns.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*sed\\s+(-i|--in-place)",
|
||||
tool: "edit",
|
||||
message: "Use the `edit` tool instead of sed -i. It provides diff preview and fuzzy matching.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*perl\\s+.*-[pn]?i",
|
||||
tool: "edit",
|
||||
message: "Use the `edit` tool instead of perl -i. It provides diff preview and fuzzy matching.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*awk\\s+.*-i\\s+inplace",
|
||||
tool: "edit",
|
||||
message: "Use the `edit` tool instead of awk -i inplace. It provides diff preview and fuzzy matching.",
|
||||
},
|
||||
{
|
||||
pattern: "^\\s*(echo|printf|cat\\s*<<)\\s+.*[^|]>\\s*\\S",
|
||||
tool: "write",
|
||||
message: "Use the `write` tool instead of echo/cat redirection. It handles encoding and provides confirmation.",
|
||||
},
|
||||
];
|
||||
import { type BashInterceptorRule, DEFAULT_BASH_INTERCEPTOR_RULES } from "../config/settings-schema";
|
||||
|
||||
export interface InterceptionResult {
|
||||
/** If true, the bash command should be blocked */
|
||||
|
||||
@@ -253,18 +253,29 @@ describe("applyHashlineEdits — replace", () => {
|
||||
expect(result.firstChangedLine).toBe(2);
|
||||
});
|
||||
|
||||
it("range replace (shrink)", () => {
|
||||
it("range replace (shrink) — end is exclusive", () => {
|
||||
const content = "aaa\nbbb\nccc\nddd";
|
||||
// end points to "ccc" which survives; only line 2 ("bbb") is consumed
|
||||
const edits: HashlineEdit[] = [{ op: "replace", pos: makeTag(2, "bbb"), end: makeTag(3, "ccc"), lines: ["ONE"] }];
|
||||
|
||||
const result = applyHashlineEdits(content, edits);
|
||||
expect(result.lines).toBe("aaa\nONE\nccc\nddd");
|
||||
});
|
||||
|
||||
it("range replace consuming multiple lines", () => {
|
||||
const content = "aaa\nbbb\nccc\nddd";
|
||||
// end points to "ddd" which survives; lines 2-3 are consumed
|
||||
const edits: HashlineEdit[] = [{ op: "replace", pos: makeTag(2, "bbb"), end: makeTag(4, "ddd"), lines: ["ONE"] }];
|
||||
|
||||
const result = applyHashlineEdits(content, edits);
|
||||
expect(result.lines).toBe("aaa\nONE\nddd");
|
||||
});
|
||||
|
||||
it("range replace (same count)", () => {
|
||||
const content = "aaa\nbbb\nccc\nddd";
|
||||
// Consume lines 2-3, replace with two lines. "ddd" survives.
|
||||
const edits: HashlineEdit[] = [
|
||||
{ op: "replace", pos: makeTag(2, "bbb"), end: makeTag(3, "ccc"), lines: ["XXX", "YYY"] },
|
||||
{ op: "replace", pos: makeTag(2, "bbb"), end: makeTag(4, "ddd"), lines: ["XXX", "YYY"] },
|
||||
];
|
||||
|
||||
const result = applyHashlineEdits(content, edits);
|
||||
@@ -272,6 +283,28 @@ describe("applyHashlineEdits — replace", () => {
|
||||
expect(result.firstChangedLine).toBe(2);
|
||||
});
|
||||
|
||||
it("applies prepend at the surviving end boundary before the enclosing range replace", () => {
|
||||
const content = "a\nb\nc\nd";
|
||||
const edits: HashlineEdit[] = [
|
||||
{ op: "replace", pos: makeTag(2, "b"), end: makeTag(4, "d"), lines: ["X"] },
|
||||
{ op: "prepend", pos: makeTag(4, "d"), lines: ["Y"] },
|
||||
];
|
||||
const result = applyHashlineEdits(content, edits);
|
||||
expect(result.lines).toBe("a\nX\nY\nd");
|
||||
expect(result.firstChangedLine).toBe(2);
|
||||
});
|
||||
|
||||
it("applies single-line replace at the surviving end boundary before the enclosing range replace", () => {
|
||||
const content = "a\nb\nc\nd";
|
||||
const edits: HashlineEdit[] = [
|
||||
{ op: "replace", pos: makeTag(2, "b"), end: makeTag(4, "d"), lines: ["X"] },
|
||||
{ op: "replace", pos: makeTag(4, "d"), lines: ["Y"] },
|
||||
];
|
||||
const result = applyHashlineEdits(content, edits);
|
||||
expect(result.lines).toBe("a\nX\nY");
|
||||
expect(result.firstChangedLine).toBe(2);
|
||||
});
|
||||
|
||||
it("replaces first line", () => {
|
||||
const content = "first\nsecond\nthird";
|
||||
const edits: HashlineEdit[] = [{ op: "replace", pos: makeTag(1, "first"), lines: ["FIRST"] }];
|
||||
@@ -305,9 +338,10 @@ describe("applyHashlineEdits — delete", () => {
|
||||
expect(result.firstChangedLine).toBe(2);
|
||||
});
|
||||
|
||||
it("deletes range of lines", () => {
|
||||
it("deletes range of lines (end exclusive)", () => {
|
||||
const content = "aaa\nbbb\nccc\nddd";
|
||||
const edits: HashlineEdit[] = [{ op: "replace", pos: makeTag(2, "bbb"), end: makeTag(3, "ccc"), lines: [] }];
|
||||
// end points to "ddd" which survives; lines 2-3 deleted
|
||||
const edits: HashlineEdit[] = [{ op: "replace", pos: makeTag(2, "bbb"), end: makeTag(4, "ddd"), lines: [] }];
|
||||
|
||||
const result = applyHashlineEdits(content, edits);
|
||||
expect(result.lines).toBe("aaa\nddd");
|
||||
@@ -494,11 +528,12 @@ describe("applyHashlineEdits — heuristics", () => {
|
||||
|
||||
it("does not override model whitespace choices in replacement content", () => {
|
||||
const content = ["import { foo } from 'x';", "import { bar } from 'y';", "const x = 1;"].join("\n");
|
||||
// end points to "const x = 1;" (survives); lines 1-2 are consumed
|
||||
const edits: HashlineEdit[] = [
|
||||
{
|
||||
op: "replace",
|
||||
pos: makeTag(1, "import { foo } from 'x';"),
|
||||
end: makeTag(2, "import { bar } from 'y';"),
|
||||
end: makeTag(3, "const x = 1;"),
|
||||
lines: ["import {foo} from 'x';", "import { bar } from 'y';", "// added"],
|
||||
},
|
||||
];
|
||||
@@ -511,21 +546,21 @@ describe("applyHashlineEdits — heuristics", () => {
|
||||
expect(outLines[3]).toBe("const x = 1;");
|
||||
});
|
||||
|
||||
it("treats same-line ranges as single-line replacements", () => {
|
||||
it("rejects same-line range (pos must be < end)", () => {
|
||||
const content = "aaa\nbbb\nccc";
|
||||
const good = makeTag(2, "bbb");
|
||||
const edits: HashlineEdit[] = [{ op: "replace", pos: good, end: good, lines: ["BBB"] }];
|
||||
const result = applyHashlineEdits(content, edits);
|
||||
expect(result.lines).toBe("aaa\nBBB\nccc");
|
||||
expect(() => applyHashlineEdits(content, edits)).toThrow(/must be < end/);
|
||||
});
|
||||
|
||||
it("auto-corrects off-by-one range end that duplicates a closing brace", () => {
|
||||
it("auto-corrects when model re-emits the exclusive end line (closing brace)", () => {
|
||||
const content = "if (ok) {\n run();\n}\nafter();";
|
||||
// end points to "}" which should survive. Model accidentally includes it in lines.
|
||||
const edits: HashlineEdit[] = [
|
||||
{
|
||||
op: "replace",
|
||||
pos: makeTag(1, "if (ok) {"),
|
||||
end: makeTag(2, " run();"),
|
||||
end: makeTag(3, "}"),
|
||||
lines: ["if (ok) {", " runSafe();", "}"],
|
||||
},
|
||||
];
|
||||
@@ -536,13 +571,14 @@ describe("applyHashlineEdits — heuristics", () => {
|
||||
expect(result.warnings?.[0]).toContain('"}"');
|
||||
});
|
||||
|
||||
it('auto-corrects off-by-one range end that duplicates a ");" closer', () => {
|
||||
it('auto-corrects when model re-emits the exclusive end line (");" closer)', () => {
|
||||
const content = "doThing(\n value,\n);\nnext();";
|
||||
// end points to ");" which should survive. Model accidentally includes it.
|
||||
const edits: HashlineEdit[] = [
|
||||
{
|
||||
op: "replace",
|
||||
pos: makeTag(1, "doThing("),
|
||||
end: makeTag(2, " value,"),
|
||||
end: makeTag(3, ");"),
|
||||
lines: ["doThing(", " normalize(value),", ");"],
|
||||
},
|
||||
];
|
||||
@@ -552,13 +588,14 @@ describe("applyHashlineEdits — heuristics", () => {
|
||||
expect(result.warnings?.[0]).toContain('");"');
|
||||
});
|
||||
|
||||
it("auto-corrects duplicated trailing lines when they match the next surviving line", () => {
|
||||
it("auto-corrects when model re-emits the exclusive end line (generic content)", () => {
|
||||
const content = "start\n oldCall();\nnextCall();\nafter();";
|
||||
// end points to "nextCall();" which should survive. Model includes it in lines.
|
||||
const edits: HashlineEdit[] = [
|
||||
{
|
||||
op: "replace",
|
||||
pos: makeTag(1, "start"),
|
||||
end: makeTag(2, " oldCall();"),
|
||||
end: makeTag(3, "nextCall();"),
|
||||
lines: ["start", " newCall();", "nextCall();"],
|
||||
},
|
||||
];
|
||||
@@ -570,15 +607,18 @@ describe("applyHashlineEdits — heuristics", () => {
|
||||
|
||||
it("auto-corrects off-by-one range start that duplicates a preceding line", () => {
|
||||
const content = "if (x) {\n oldBody();\n}\nafter();";
|
||||
// pos is body line 2, end is "}" (exclusive, survives).
|
||||
// Model includes "if (x) {" in lines, duplicating preceding line.
|
||||
const edits: HashlineEdit[] = [
|
||||
{
|
||||
op: "replace",
|
||||
pos: makeTag(2, " oldBody();"),
|
||||
end: makeTag(3, "}"),
|
||||
lines: ["if (x) {", " newBody();", "}"],
|
||||
lines: ["if (x) {", " newBody();"],
|
||||
},
|
||||
];
|
||||
const result = applyHashlineEdits(content, edits);
|
||||
// Leading "if (x) {" is popped; "}" survives from exclusive end
|
||||
expect(result.lines).toBe("if (x) {\n newBody();\n}\nafter();");
|
||||
expect(result.warnings).toHaveLength(1);
|
||||
expect(result.warnings?.[0]).toContain("removed leading replacement line");
|
||||
@@ -685,11 +725,12 @@ describe("applyHashlineEdits — multiple edits", () => {
|
||||
|
||||
it("applies non-overlapping edits against original anchors when line counts change", () => {
|
||||
const content = "one\ntwo\nthree\nfour\nfive\nsix";
|
||||
// end is exclusive: "four" survives, lines 2-3 are consumed
|
||||
const edits: HashlineEdit[] = [
|
||||
{
|
||||
op: "replace",
|
||||
pos: makeTag(2, "two"),
|
||||
end: makeTag(3, "three"),
|
||||
end: makeTag(4, "four"),
|
||||
lines: ["TWO_THREE"],
|
||||
},
|
||||
{ op: "replace", pos: makeTag(6, "six"), lines: ["SIX"] },
|
||||
@@ -802,7 +843,7 @@ describe("applyHashlineEdits — errors", () => {
|
||||
expect(() => applyHashlineEdits(content, edits)).toThrow(/does not exist/);
|
||||
});
|
||||
|
||||
it("rejects range with start > end", () => {
|
||||
it("rejects range with start >= end (exclusive end)", () => {
|
||||
const content = "aaa\nbbb\nccc\nddd\neee";
|
||||
const edits: HashlineEdit[] = [{ op: "replace", pos: makeTag(5, "eee"), end: makeTag(2, "bbb"), lines: ["X"] }];
|
||||
|
||||
|
||||
@@ -1,19 +1,11 @@
|
||||
import { afterEach, describe, expect, it, mock, vi } from "bun:test";
|
||||
import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test";
|
||||
import { SessionSelectorComponent } from "../../../src/modes/components/session-selector";
|
||||
import { initTheme } from "../../../src/modes/theme/theme";
|
||||
import type { SessionInfo } from "../../../src/session/session-manager";
|
||||
|
||||
const themeModulePath = new URL("../../../src/modes/theme/theme.ts", import.meta.url).pathname;
|
||||
|
||||
mock.module(themeModulePath, () => ({
|
||||
theme: {
|
||||
fg: (_tone: string, text: string) => text,
|
||||
bold: (text: string) => text,
|
||||
nav: { cursor: ">" },
|
||||
sep: { dot: "·" },
|
||||
boxSharp: { horizontal: "-", vertical: "|" },
|
||||
},
|
||||
}));
|
||||
|
||||
import { SessionSelectorComponent } from "../../../src/modes/components/session-selector";
|
||||
beforeAll(() => {
|
||||
initTheme();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
|
||||
@@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { DEFAULT_BASH_INTERCEPTOR_RULES, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { EditTool } from "@oh-my-pi/pi-coding-agent/patch";
|
||||
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
|
||||
import { BashTool } from "@oh-my-pi/pi-coding-agent/tools/bash";
|
||||
@@ -455,6 +455,91 @@ function b() {
|
||||
expect(result.details).toBeUndefined();
|
||||
});
|
||||
|
||||
it("should expose built-in interceptor defaults truthfully", () => {
|
||||
const defaultSettings = Settings.isolated({ "bashInterceptor.enabled": true });
|
||||
const explicitEmptySettings = Settings.isolated({
|
||||
"bashInterceptor.enabled": true,
|
||||
"bashInterceptor.patterns": [],
|
||||
});
|
||||
|
||||
expect(defaultSettings.get("bashInterceptor.patterns")).toEqual(DEFAULT_BASH_INTERCEPTOR_RULES);
|
||||
expect(defaultSettings.getBashInterceptorRules()).toEqual(DEFAULT_BASH_INTERCEPTOR_RULES);
|
||||
expect(explicitEmptySettings.get("bashInterceptor.patterns")).toEqual([]);
|
||||
expect(explicitEmptySettings.getBashInterceptorRules()).toEqual([]);
|
||||
});
|
||||
|
||||
it("should block built-in interceptor commands when enabled with default patterns", async () => {
|
||||
const interceptedBashTool = wrapToolWithMetaNotice(
|
||||
new BashTool(createTestToolSession(testDir, Settings.isolated({ "bashInterceptor.enabled": true }))),
|
||||
);
|
||||
|
||||
await expect(
|
||||
interceptedBashTool.execute(
|
||||
"test-call-8-intercept-default",
|
||||
{ command: "cat test.txt" },
|
||||
undefined,
|
||||
undefined,
|
||||
{ toolNames: ["read"] },
|
||||
),
|
||||
).rejects.toThrow(/Use the `read` tool instead of cat\/head\/tail/);
|
||||
});
|
||||
|
||||
it("should allow an explicit empty interceptor pattern list", async () => {
|
||||
const allowedFile = path.join(testDir, "allow-empty.txt");
|
||||
fs.writeFileSync(allowedFile, "empty means empty\n");
|
||||
|
||||
const interceptedBashTool = wrapToolWithMetaNotice(
|
||||
new BashTool(
|
||||
createTestToolSession(
|
||||
testDir,
|
||||
Settings.isolated({
|
||||
"bashInterceptor.enabled": true,
|
||||
"bashInterceptor.patterns": [],
|
||||
}),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
const result = await interceptedBashTool.execute(
|
||||
"test-call-8-intercept-empty",
|
||||
{ command: `cat ${allowedFile}` },
|
||||
undefined,
|
||||
undefined,
|
||||
{ toolNames: ["read"] },
|
||||
);
|
||||
|
||||
expect(getTextOutput(result)).toContain("empty means empty");
|
||||
});
|
||||
|
||||
it("should honor custom bash interceptor patterns", async () => {
|
||||
const interceptedBashTool = wrapToolWithMetaNotice(
|
||||
new BashTool(
|
||||
createTestToolSession(
|
||||
testDir,
|
||||
Settings.isolated({
|
||||
"bashInterceptor.enabled": true,
|
||||
"bashInterceptor.patterns": [
|
||||
{
|
||||
pattern: "^\\s*customcmd\\s+",
|
||||
tool: "grep",
|
||||
message: "Use the `grep` tool for customcmd.",
|
||||
},
|
||||
],
|
||||
}),
|
||||
),
|
||||
),
|
||||
);
|
||||
await expect(
|
||||
interceptedBashTool.execute(
|
||||
"test-call-8-intercept-custom",
|
||||
{ command: "customcmd foo" },
|
||||
undefined,
|
||||
undefined,
|
||||
{ toolNames: ["grep"] },
|
||||
),
|
||||
).rejects.toThrow(/Use the `grep` tool for customcmd\./);
|
||||
});
|
||||
|
||||
it("should expose env values without shell re-parsing", async () => {
|
||||
const mermaid = [
|
||||
"flowchart TD",
|
||||
|
||||
Reference in New Issue
Block a user