shumengya/sproutclaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): tighten skill discovery and edit diffs closes #2603

Browse Source
This commit is contained in:
Mario Zechner
2026-03-27 03:48:28 +01:00
parent eeace79715
commit a0734bd162
6 changed files with 168 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
packages/coding-agent/CHANGELOG.md
View File

@@ -6,6 +6,8 @@
- Fixed repeated compactions dropping messages that were kept by an earlier compaction by re-summarizing from the previous kept boundary and recalculating `tokensBefore` from the rebuilt session context ([#2608](https://github.com/badlogic/pi-mono/issues/2608))
- Fixed interactive compaction UI updates so `ctx.compact()` rebuilds the chat through unified compaction events, manual compaction no longer duplicates the summary block, and the `trigger-compact` example only fires when context usage crosses its threshold ([#2617](https://github.com/badlogic/pi-mono/issues/2617))
- Fixed skill discovery to stop recursing once a directory contains `SKILL.md`, and to ignore root `*.md` files in `.agents/skills` while keeping root markdown skill files supported in `~/.pi/agent/skills`, `.pi/skills`, and package `skills/` directories ([#2603](https://github.com/badlogic/pi-mono/issues/2603))
- Fixed edit tool diff rendering for multi-edit operations with large unchanged gaps so distant edits collapse intermediate context instead of dumping the full unchanged middle block
- Fixed auto-compaction overflow recovery for Ollama models when the backend returns explicit `prompt too long; exceeded max context length ...` errors instead of silently truncating input ([#2626](https://github.com/badlogic/pi-mono/issues/2626))
- Fixed built-in tool overrides that reuse built-in parameter schemas to still honor custom `renderCall` and `renderResult` renderers in the interactive TUI, restoring the `minimal-mode` example ([#2595](https://github.com/badlogic/pi-mono/issues/2595))

5
packages/coding-agent/docs/skills.md
View File

@@ -34,8 +34,9 @@ Pi loads skills from:
- CLI: `--skill <path>` (repeatable, additive even with `--no-skills`)
Discovery rules:
- Direct `.md` files in the skills directory root
- Recursive `SKILL.md` files under subdirectories
- In `~/.pi/agent/skills/` and `.pi/skills/`, direct root `.md` files are discovered as individual skills
- In all skill locations, directories containing `SKILL.md` are discovered recursively
- In `~/.agents/skills/` and project `.agents/skills/`, root `.md` files are ignored
Disable discovery with `--no-skills` (explicit `--skill` paths still load).

58
packages/coding-agent/src/core/package-manager.ts
View File

@@ -249,9 +249,11 @@ function collectFiles(
return files;
}
type SkillDiscoveryMode = "pi" | "agents";
function collectSkillEntries(
dir: string,
includeRootFiles = true,
mode: SkillDiscoveryMode,
ignoreMatcher?: IgnoreMatcher,
rootDir?: string,
): string[] {
@@ -264,6 +266,29 @@ function collectSkillEntries(
try {
const dirEntries = readdirSync(dir, { withFileTypes: true });
for (const entry of dirEntries) {
if (entry.name !== "SKILL.md") {
continue;
}
const fullPath = join(dir, entry.name);
let isFile = entry.isFile();
if (entry.isSymbolicLink()) {
try {
isFile = statSync(fullPath).isFile();
} catch {
continue;
}
}
const relPath = toPosixPath(relative(root, fullPath));
if (isFile && !ig.ignores(relPath)) {
entries.push(fullPath);
return entries;
}
}
for (const entry of dirEntries) {
if (entry.name.startsWith(".")) continue;
if (entry.name === "node_modules") continue;
@@ -283,18 +308,15 @@ function collectSkillEntries(
}
const relPath = toPosixPath(relative(root, fullPath));
const ignorePath = isDir ? `${relPath}/` : relPath;
if (ig.ignores(ignorePath)) continue;
if (isDir) {
entries.push(...collectSkillEntries(fullPath, false, ig, root));
} else if (isFile) {
const isRootMd = includeRootFiles && entry.name.endsWith(".md");
const isSkillMd = !includeRootFiles && entry.name === "SKILL.md";
if (isRootMd || isSkillMd) {
entries.push(fullPath);
}
if (mode === "pi" && dir === root && isFile && entry.name.endsWith(".md") && !ig.ignores(relPath)) {
entries.push(fullPath);
continue;
}
if (!isDir) continue;
if (ig.ignores(`${relPath}/`)) continue;
entries.push(...collectSkillEntries(fullPath, mode, ig, root));
}
} catch {
// Ignore errors
@@ -303,8 +325,8 @@ function collectSkillEntries(
return entries;
}
function collectAutoSkillEntries(dir: string, includeRootFiles = true): string[] {
return collectSkillEntries(dir, includeRootFiles);
function collectAutoSkillEntries(dir: string, mode: SkillDiscoveryMode): string[] {
return collectSkillEntries(dir, mode);
}
function findGitRepoRoot(startDir: string): string | null {
@@ -516,7 +538,7 @@ function collectAutoExtensionEntries(dir: string): string[] {
*/
function collectResourceFiles(dir: string, resourceType: ResourceType): string[] {
if (resourceType === "skills") {
return collectSkillEntries(dir);
return collectSkillEntries(dir, "pi");
}
if (resourceType === "extensions") {
return collectAutoExtensionEntries(dir);
@@ -1964,8 +1986,8 @@ export class DefaultPackageManager implements PackageManager {
addResources(
"skills",
[
...collectAutoSkillEntries(projectDirs.skills),
...projectAgentsSkillDirs.flatMap((dir) => collectAutoSkillEntries(dir)),
...collectAutoSkillEntries(projectDirs.skills, "pi"),
...projectAgentsSkillDirs.flatMap((dir) => collectAutoSkillEntries(dir, "agents")),
],
projectMetadata,
projectOverrides.skills,
@@ -1995,7 +2017,7 @@ export class DefaultPackageManager implements PackageManager {
);
addResources(
"skills",
[...collectAutoSkillEntries(userDirs.skills), ...collectAutoSkillEntries(userAgentsSkillsDir)],
[...collectAutoSkillEntries(userDirs.skills, "pi"), ...collectAutoSkillEntries(userAgentsSkillsDir, "agents")],
userMetadata,
userOverrides.skills,
globalBaseDir,

79
packages/coding-agent/src/core/tools/edit-diff.ts
View File

@@ -311,46 +311,69 @@ export function generateDiffString(
} else {
// Context lines - only show a few before/after changes
const nextPartIsChange = i < parts.length - 1 && (parts[i + 1].added || parts[i + 1].removed);
const hasLeadingChange = lastWasChange;
const hasTrailingChange = nextPartIsChange;
if (lastWasChange || nextPartIsChange) {
// Show context
let linesToShow = raw;
let skipStart = 0;
let skipEnd = 0;
if (hasLeadingChange && hasTrailingChange) {
if (raw.length <= contextLines * 2) {
for (const line of raw) {
const lineNum = String(oldLineNum).padStart(lineNumWidth, " ");
output.push(` ${lineNum} ${line}`);
oldLineNum++;
newLineNum++;
}
} else {
const leadingLines = raw.slice(0, contextLines);
const trailingLines = raw.slice(raw.length - contextLines);
const skippedLines = raw.length - leadingLines.length - trailingLines.length;
if (!lastWasChange) {
// Show only last N lines as leading context
skipStart = Math.max(0, raw.length - contextLines);
linesToShow = raw.slice(skipStart);
}
for (const line of leadingLines) {
const lineNum = String(oldLineNum).padStart(lineNumWidth, " ");
output.push(` ${lineNum} ${line}`);
oldLineNum++;
newLineNum++;
}
if (!nextPartIsChange && linesToShow.length > contextLines) {
// Show only first N lines as trailing context
skipEnd = linesToShow.length - contextLines;
linesToShow = linesToShow.slice(0, contextLines);
}
// Add ellipsis if we skipped lines at start
if (skipStart > 0) {
output.push(` ${"".padStart(lineNumWidth, " ")} ...`);
// Update line numbers for the skipped leading context
oldLineNum += skipStart;
newLineNum += skipStart;
}
oldLineNum += skippedLines;
newLineNum += skippedLines;
for (const line of linesToShow) {
for (const line of trailingLines) {
const lineNum = String(oldLineNum).padStart(lineNumWidth, " ");
output.push(` ${lineNum} ${line}`);
oldLineNum++;
newLineNum++;
}
}
} else if (hasLeadingChange) {
const shownLines = raw.slice(0, contextLines);
const skippedLines = raw.length - shownLines.length;
for (const line of shownLines) {
const lineNum = String(oldLineNum).padStart(lineNumWidth, " ");
output.push(` ${lineNum} ${line}`);
oldLineNum++;
newLineNum++;
}
// Add ellipsis if we skipped lines at end
if (skipEnd > 0) {
if (skippedLines > 0) {
output.push(` ${"".padStart(lineNumWidth, " ")} ...`);
// Update line numbers for the skipped trailing context
oldLineNum += skipEnd;
newLineNum += skipEnd;
oldLineNum += skippedLines;
newLineNum += skippedLines;
}
} else if (hasTrailingChange) {
const skippedLines = Math.max(0, raw.length - contextLines);
if (skippedLines > 0) {
output.push(` ${"".padStart(lineNumWidth, " ")} ...`);
oldLineNum += skippedLines;
newLineNum += skippedLines;
}
for (const line of raw.slice(skippedLines)) {
const lineNum = String(oldLineNum).padStart(lineNumWidth, " ");
output.push(` ${lineNum} ${line}`);
oldLineNum++;
newLineNum++;
}
} else {
// Skip these context lines entirely

49
packages/coding-agent/test/package-manager.test.ts
View File

@@ -106,6 +106,22 @@ Content`,
expect(result.skills.some((r) => r.path === skillFile && r.enabled)).toBe(true);
});
it("should auto-discover root markdown skills from .pi skill dirs", async () => {
const skillFile = join(agentDir, "skills", "single-file.md");
mkdirSync(join(agentDir, "skills"), { recursive: true });
writeFileSync(
skillFile,
`---
name: single-file
description: A root markdown skill
---
Content`,
);
const result = await packageManager.resolve();
expect(result.skills.some((r) => r.path === skillFile && r.enabled)).toBe(true);
});
it("should resolve project paths relative to .pi", async () => {
const extDir = join(tempDir, ".pi", "extensions");
mkdirSync(extDir, { recursive: true });
@@ -232,6 +248,26 @@ Content`,
expect(result.skills.some((r) => r.path === middleSkill && r.enabled)).toBe(true);
});
it("should ignore root markdown files in .agents/skills", async () => {
const agentsSkillsDir = join(tempDir, ".agents", "skills");
mkdirSync(join(agentsSkillsDir, "nested-skill"), { recursive: true });
const rootSkill = join(agentsSkillsDir, "root-file.md");
const nestedSkill = join(agentsSkillsDir, "nested-skill", "SKILL.md");
writeFileSync(rootSkill, "---\nname: root-file\ndescription: Root markdown file\n---\n");
writeFileSync(nestedSkill, "---\nname: nested-skill\ndescription: Nested skill\n---\n");
const pm = new DefaultPackageManager({
cwd: join(tempDir, "work"),
agentDir,
settingsManager,
});
mkdirSync(join(tempDir, "work"), { recursive: true });
const result = await pm.resolve();
expect(result.skills.some((r) => r.path === rootSkill)).toBe(false);
expect(result.skills.some((r) => r.path === nestedSkill && r.enabled)).toBe(true);
});
it("should keep ~/.agents/skills user-scoped when cwd is under home in a non-git directory", async () => {
const previousHome = process.env.HOME;
process.env.HOME = tempDir;
@@ -352,6 +388,19 @@ Content`,
expect(result.extensions.some((r) => pathEndsWith(r.path, "main.ts") && r.enabled)).toBe(true);
expect(result.themes.some((r) => pathEndsWith(r.path, "dark.json") && r.enabled)).toBe(true);
});
it("should stop recursing when a package skill directory contains SKILL.md", async () => {
const pkgDir = join(tempDir, "skill-root-pkg");
mkdirSync(join(pkgDir, "skills", "root-skill", "nested-skill"), { recursive: true });
const rootSkill = join(pkgDir, "skills", "root-skill", "SKILL.md");
const nestedSkill = join(pkgDir, "skills", "root-skill", "nested-skill", "SKILL.md");
writeFileSync(rootSkill, "---\nname: root-skill\ndescription: Root skill\n---\n");
writeFileSync(nestedSkill, "---\nname: nested-skill\ndescription: Nested skill\n---\n");
const result = await packageManager.resolveExtensionSources([pkgDir]);
expect(result.skills.some((r) => r.path === rootSkill && r.enabled)).toBe(true);
expect(result.skills.some((r) => r.path === nestedSkill)).toBe(false);
});
});
describe("progress callback", () => {

23
packages/coding-agent/test/tools.test.ts
View File

@@ -277,6 +277,29 @@ describe("Coding Agent Tools", () => {
expect(result.details?.diff).toContain("GAMMA");
});
it("should collapse large unchanged gaps in multi-edit diffs", async () => {
const testFile = join(testDir, "edit-multi-large-gap.txt");
const lines = Array.from({ length: 600 }, (_, i) => `line ${String(i + 1).padStart(3, "0")}`);
writeFileSync(testFile, `${lines.join("\n")}\n`);
const result = await editTool.execute("test-call-8b", {
path: testFile,
edits: [
{ oldText: "line 100\n", newText: "LINE 100\n" },
{ oldText: "line 300\n", newText: "LINE 300\n" },
{ oldText: "line 500\n", newText: "LINE 500\n" },
],
});
const diff = result.details?.diff ?? "";
expect(diff).toContain("LINE 100");
expect(diff).toContain("LINE 300");
expect(diff).toContain("LINE 500");
expect(diff).toContain("...");
expect(diff).not.toContain("line 250");
expect(diff.split("\n").length).toBeLessThan(50);
});
it("should match edits against the original file, not incrementally", async () => {
const testFile = join(testDir, "edit-multi-original.txt");
writeFileSync(testFile, "foo\nbar\nbaz\n");