Skip to content

Commit 1c56034

Browse files
Copilotpelikhan
andauthored
Detect skill install failures and surface them as agent failure context; fix needex/skills fallback crash (#42642)
* Add skill install failure detection and fix needex/skills fallback Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Address code review feedback on skill install failure detection Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Harden skill install failure serialization and reporting Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Tighten failure record validation and compatibility parsing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent fa43ff3 commit 1c56034

15 files changed

Lines changed: 298 additions & 16 deletions

.github/workflows/impeccable-skills-reviewer.lock.yml

Lines changed: 19 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/impeccable-skills-reviewer.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
private: true
33
emoji: "🧵"
44
name: "Impeccable Skills Reviewer"
5-
description: Reviews pull requests using Impeccable skills from needex/skills and applies the most relevant skills based on changed files
5+
description: Reviews pull requests using Impeccable skills and applies the most relevant skills based on changed files
66
on:
77
pull_request:
88
types: [ready_for_review]
@@ -12,7 +12,7 @@ permissions:
1212
pull-requests: read
1313
copilot-requests: write
1414
skills:
15-
- skill: ${{ vars.IMPECCABLE_SKILLS_REF || 'needex/skills' }}
15+
- skill: ${{ vars.IMPECCABLE_SKILLS_REF }}
1616
github-token: ${{ secrets.GITHUB_TOKEN }}
1717

1818
sandbox:
@@ -77,7 +77,7 @@ timeout-minutes: 15
7777

7878
# Impeccable Skills Reviewer
7979

80-
You are a pull request reviewer that uses Impeccable skills from `needex/skills`.
80+
You are a pull request reviewer that uses Impeccable skills.
8181

8282
## Mission
8383

.github/workflows/mattpocock-skills-reviewer.lock.yml

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// @ts-check
2+
/// <reference types="@actions/github-script" />
3+
4+
const fs = require("fs");
5+
6+
/** Path written by install_frontmatter_skills.cjs during the activation job. */
7+
const SKILL_FAILURES_FILE = "/tmp/gh-aw/skill_install_failures.json";
8+
9+
/**
10+
* Read the shared failures file produced by install_frontmatter_skills.cjs.
11+
* Returns an empty array when the file does not exist or cannot be parsed.
12+
* @returns {Array<{skill: string; error: string}>}
13+
*/
14+
function readSkillInstallFailures() {
15+
try {
16+
if (!fs.existsSync(SKILL_FAILURES_FILE)) {
17+
return [];
18+
}
19+
const raw = fs.readFileSync(SKILL_FAILURES_FILE, "utf8");
20+
const parsed = JSON.parse(raw);
21+
if (!Array.isArray(parsed)) {
22+
return [];
23+
}
24+
return parsed.filter(entry => entry && typeof entry.skill === "string" && typeof entry.error === "string");
25+
} catch (readErr) {
26+
// Warn so "no failures" vs "couldn't read failures file" is distinguishable in logs
27+
core.warning(`Could not read skill install failures file: ${readErr instanceof Error ? readErr.message : String(readErr)}`);
28+
return [];
29+
}
30+
}
31+
32+
async function main() {
33+
const failures = readSkillInstallFailures();
34+
const failureCount = failures.length;
35+
36+
core.info(`Skill install failures detected: ${failureCount}`);
37+
38+
core.setOutput("failure_count", String(failureCount));
39+
core.setOutput("errors", failures.map(f => `${f.skill}\t${f.error.replace(/\r?\n/g, " ").replace(/\t/g, " ")}`).join("\n"));
40+
41+
if (failureCount > 0) {
42+
core.warning(`${failureCount} skill(s) failed to install — see agent failure issue/comment for details`);
43+
}
44+
}
45+
46+
module.exports = { main, readSkillInstallFailures };
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
import fs from "fs";
3+
import { createRequire } from "module";
4+
5+
const require = createRequire(import.meta.url);
6+
const script = require("./collect_skill_install_failures.cjs");
7+
8+
describe("collect_skill_install_failures", () => {
9+
let originalCore;
10+
11+
beforeEach(() => {
12+
originalCore = global.core;
13+
global.core = {
14+
info: vi.fn(),
15+
warning: vi.fn(),
16+
setOutput: vi.fn(),
17+
};
18+
});
19+
20+
afterEach(() => {
21+
global.core = originalCore;
22+
fs.rmSync("/tmp/gh-aw/skill_install_failures.json", { force: true });
23+
});
24+
25+
it("returns empty array when failures file is missing", () => {
26+
expect(script.readSkillInstallFailures()).toEqual([]);
27+
});
28+
29+
it("filters malformed entries and emits sanitized outputs", async () => {
30+
fs.mkdirSync("/tmp/gh-aw", { recursive: true });
31+
fs.writeFileSync("/tmp/gh-aw/skill_install_failures.json", JSON.stringify([{ skill: "owner/repo", error: "line1\nline2" }, { skill: "missing-error" }]), "utf8");
32+
33+
await script.main();
34+
35+
expect(global.core.setOutput).toHaveBeenCalledWith("failure_count", "1");
36+
expect(global.core.setOutput).toHaveBeenCalledWith("errors", "owner/repo\tline1 line2");
37+
});
38+
39+
it("warns and returns empty array on unreadable failures file", () => {
40+
fs.mkdirSync("/tmp/gh-aw", { recursive: true });
41+
fs.writeFileSync("/tmp/gh-aw/skill_install_failures.json", "{invalid", "utf8");
42+
43+
expect(script.readSkillInstallFailures()).toEqual([]);
44+
expect(global.core.warning).toHaveBeenCalledWith(expect.stringContaining("Could not read skill install failures file"));
45+
});
46+
});

0 commit comments

Comments
 (0)