library: a skill or server switched off for every agent is saved with "agents": [] not null, a check's finds beside skills on no agent come with [] too, and the Library page draws a find without its agents and says why when it can't draw a check's reply, where it threw on n.agents and went blank after Check for updates (#1217)
This commit is contained in:
1 parent
c2020e0bb3
commit
bb8a3e3025
5 files changed
+211
-4
No files matched your search
@@ -1851,7 +1851,9 @@
|
||||
src.append(a);
|
||||
who.append(src);
|
||||
const have = el("div", "lib-have");
|
||||
for (const id of n.agents) { const ag = agentOf(id); if (ag) { const i = agentIcon(ag.icon); i.title = ag.name; have.append(i); } }
|
||||
// one find without its list of agents doesn't take the page down (#1217)
|
||||
const agents = n.agents || [];
|
||||
for (const id of agents) { const ag = agentOf(id); if (ag) { const i = agentIcon(ag.icon); i.title = ag.name; have.append(i); } }
|
||||
const ign = button(t("Ignore"), "", () => change("skills/ignore-new", { names: [n.id] }, t("{name} set aside", { name: n.name })));
|
||||
ign.title = t("A check for updates won't offer it again");
|
||||
const add = button(t("Add"), "action", async (e, b) => {
|
||||
@@ -1859,7 +1861,7 @@
|
||||
await change("skills/add-new", { names: [n.id] }, t("{name} is in the library now", { name: n.name }));
|
||||
b.classList.remove("busy");
|
||||
});
|
||||
const names = n.agents.map(nameOf).join(", ");
|
||||
const names = agents.map(nameOf).join(", ");
|
||||
add.title = names ? t("Adds it to the library for {agents}, which have its repository's other skills", { agents: names }) : t("Adds it to the library");
|
||||
row.append(glyph(GLYPH.skill), who, have, ign, add);
|
||||
return row;
|
||||
@@ -3194,7 +3196,9 @@
|
||||
status(e.message, "err", 6000);
|
||||
}
|
||||
checking = false;
|
||||
render();
|
||||
// drawn after the await, a page that can't be drawn says why, as one
|
||||
// loaded does, not left blank without a word (#1217)
|
||||
try { render(); } catch (e) { status(e.message, "err"); }
|
||||
}
|
||||
|
||||
// why a skill couldn't be checked: GitHub's rate limit used up said in
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
// Run with Node's test runner and Playwright on the module path; see README.md.
|
||||
// char1eslu (#1217): skills from majiayu000/claude-arsenal,
|
||||
// anthropics/claude-plugins-community and typesafe-ai/skills were on no
|
||||
// agent, and magpie sent the skills a check found beside them with
|
||||
// "agents": null. Drawing one threw ("null is not an object (evaluating
|
||||
// 'id of n.agents')"): on load the page said so in red and stayed empty,
|
||||
// and after "Check for updates" (技能库 → 检查更新) the page went blank
|
||||
// without a word. A find without its agents is drawn as on no agent; a
|
||||
// reply the page can't draw after a check says why. In English and
|
||||
// Chinese, wide and narrow, in Chromium and WebKit. No backend: the API is
|
||||
// faked here.
|
||||
const assert = require("node:assert/strict");
|
||||
const fs = require("node:fs/promises");
|
||||
const path = require("node:path");
|
||||
const { test } = require("node:test");
|
||||
const { chromium, webkit } = require("playwright");
|
||||
|
||||
const assets = path.resolve(__dirname, "../assets");
|
||||
const HOME = "/Users/char1eslu";
|
||||
const agent = (id, name) => ({ id, name, icon: "", skills: `${HOME}/.${id}/skills` });
|
||||
const gh = (repo, p) => `https://github.com/${repo}/tree/HEAD/${p}`;
|
||||
// the reporter's skills on no agent, beside ones that are on some
|
||||
const sk = (name, repo, p, agents) => ({ name, description: name + " skill", kind: "github", source: gh(repo, p), repo, agents, check: { name, status: "current" } });
|
||||
const skills = [
|
||||
sk("codex-fluent", "majiayu000/claude-arsenal", "skills/codex-fluent", []),
|
||||
sk("codex-retrospective", "majiayu000/claude-arsenal", "skills/codex-retrospective", []),
|
||||
sk("skill-usage-stats", "majiayu000/claude-arsenal", "skills/skill-usage-stats", []),
|
||||
sk("eli5", "anthropics/claude-plugins-community", "plugins/eli5/skills/eli5", []),
|
||||
sk("typesafe-ai", "typesafe-ai/skills", "skills/typesafe-ai", []),
|
||||
sk("pdf", "anthropics/skills", "skills/pdf", ["claude", "codex"]),
|
||||
];
|
||||
// what a check found beside them, as magpie before the fix sent it; the
|
||||
// reporter didn't name the skill, so this one stands in for it
|
||||
const REPO = "majiayu000/claude-arsenal";
|
||||
const found = { id: gh(REPO, "skills/codex-handoff"), name: "codex-handoff", description: "Hands off", repo: REPO, path: "skills/codex-handoff", from: gh(REPO, "skills"), agents: null };
|
||||
const lib = (st) => ({
|
||||
dir: `${HOME}/.config/magpie/library`, backups: `${HOME}/.config/magpie/backups`, home: HOME,
|
||||
agents: [agent("claude", "Claude Code"), agent("codex", "Codex")],
|
||||
instructions: { agents: [] }, servers: [], skills, foundServers: [], projects: [], foundSkills: [], skillGroups: [],
|
||||
newSkills: st.checked || st.foundAtLoad ? [found] : [],
|
||||
});
|
||||
|
||||
function server(lang, st) {
|
||||
return async (route) => {
|
||||
const req = route.request(), url = new URL(req.url());
|
||||
if (url.pathname === "/boot.js") return route.fulfill({ contentType: "text/javascript", body: `window.bootPrefs = {lang:"${lang}",theme:"light",web:true};` });
|
||||
if (url.pathname === "/wails/runtime.js") return route.fulfill({ contentType: "text/javascript", body: "export const Window = {};" });
|
||||
if (url.pathname === "/api/state") return route.fulfill({ json: { agents: [], profiles: [], settings: { lang, theme: "light" } } });
|
||||
if (url.pathname === "/api/library") return route.fulfill({ json: lib(st) });
|
||||
if (url.pathname === "/api/library/skills/check") {
|
||||
st.checked = true;
|
||||
const v = lib(st);
|
||||
if (st.undrawable) delete v.servers; // a reply the page can't draw
|
||||
return route.fulfill({ json: v });
|
||||
}
|
||||
if (url.pathname === "/api/plugins") return route.fulfill({ json: { plugins: [] } });
|
||||
if (url.pathname === "/api/usage/quotas") return route.fulfill({ json: [] });
|
||||
if (url.pathname === "/api/groups") return route.fulfill({ json: { groups: [] } });
|
||||
if (url.pathname === "/api/providers") return route.fulfill({ json: { providers: [], gateway: { running: true } } });
|
||||
if (url.pathname.startsWith("/api/")) return route.fulfill({ json: {} });
|
||||
const file = path.join(assets, url.pathname === "/" ? "index.html" : url.pathname);
|
||||
const contentType = { ".html": "text/html", ".js": "text/javascript", ".css": "text/css", ".svg": "image/svg+xml", ".png": "image/png" }[path.extname(file)];
|
||||
await route.fulfill({ body: await fs.readFile(file), contentType });
|
||||
};
|
||||
}
|
||||
|
||||
const words = {
|
||||
en: { check: "Check for updates", said: "Every skill is up to date · 1 more skill in their repositories", addTip: "Adds it to the library" },
|
||||
zh: { check: "检查更新", said: "所有技能都是最新的 · 同一仓库里还有 1 个技能", addTip: "加入资源库" },
|
||||
};
|
||||
|
||||
for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) {
|
||||
test(engine + ": a found skill with agents null doesn't blank the Library page", async (t) => {
|
||||
assert(["chromium", "webkit"].includes(engine), "BROWSER must be chromium or webkit");
|
||||
const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" }));
|
||||
t.after(() => browser.close());
|
||||
|
||||
const open = async (lang, width, st, errors) => {
|
||||
const ctx = await browser.newContext({ viewport: { width, height: 1100 } });
|
||||
await ctx.addInitScript(() => { try { localStorage.setItem("magpie.libTab", "skills"); } catch {} });
|
||||
const page = await ctx.newPage();
|
||||
page.setDefaultTimeout(5000);
|
||||
page.on("pageerror", (e) => errors.push(e.message));
|
||||
await page.route("**/*", server(lang, st));
|
||||
await page.goto("http://magpie.test/");
|
||||
await page.locator('button[data-view="library"]').click();
|
||||
return { ctx, page, v: page.locator("#view-library") };
|
||||
};
|
||||
const drawn = async (page, v, w, width) => {
|
||||
const row = v.locator(".lib-newskill");
|
||||
assert.equal(await row.count(), 1, "the find is listed");
|
||||
assert.equal((await row.locator(".name").textContent()).trim(), "codex-handoff");
|
||||
assert.equal(await row.locator(".lib-have > *").count(), 0, "on no agent");
|
||||
assert.equal(await row.locator("button.action").getAttribute("title"), w.addTip);
|
||||
assert(await v.locator(".lib-row", { hasText: "codex-fluent" }).count() > 0, "the library's own skills are drawn too");
|
||||
assert.equal(await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth), true, `no sideways scroll at ${width}px`);
|
||||
};
|
||||
|
||||
for (const width of [860, 440]) for (const lang of ["en", "zh"]) {
|
||||
const w = words[lang];
|
||||
|
||||
await t.test(`${lang} at ${width}px: Check for updates`, async () => {
|
||||
const errors = [], st = {};
|
||||
const { ctx, page, v } = await open(lang, width, st, errors);
|
||||
await v.locator(".lib-body:not(.lib-skel) .lib-row").first().waitFor();
|
||||
await v.locator(".row-head button.lib-updall", { hasText: w.check }).click();
|
||||
await page.waitForFunction(() => !document.querySelector("#view-library .lib-updall.busy, #view-library .lib-updall[disabled]"));
|
||||
await page.waitForTimeout(200);
|
||||
assert.deepEqual(errors, []);
|
||||
assert.equal((await page.locator("#status").textContent()).trim(), w.said);
|
||||
await drawn(page, v, w, width);
|
||||
await ctx.close();
|
||||
});
|
||||
|
||||
await t.test(`${lang} at ${width}px: loaded after a check`, async () => {
|
||||
const errors = [], st = { foundAtLoad: true };
|
||||
const { ctx, page, v } = await open(lang, width, st, errors);
|
||||
await v.locator(".lib-body:not(.lib-skel) .lib-row").first().waitFor();
|
||||
await page.waitForTimeout(200);
|
||||
assert.deepEqual(errors, []);
|
||||
assert.notEqual(await page.locator("#status").getAttribute("class"), "status err", "no error said");
|
||||
await drawn(page, v, w, width);
|
||||
await ctx.close();
|
||||
});
|
||||
|
||||
await t.test(`${lang} at ${width}px: a check's reply the page can't draw says so`, async () => {
|
||||
const errors = [], st = { undrawable: true };
|
||||
const { ctx, page, v } = await open(lang, width, st, errors);
|
||||
await v.locator(".lib-body:not(.lib-skel) .lib-row").first().waitFor();
|
||||
await v.locator(".row-head button.lib-updall", { hasText: w.check }).click();
|
||||
await page.waitForTimeout(500);
|
||||
assert.deepEqual(errors, [], "no uncaught error");
|
||||
const s = page.locator("#status");
|
||||
assert.equal(await s.getAttribute("class"), "status err");
|
||||
assert.notEqual((await s.textContent()).trim(), w.said, "not said to have gone well");
|
||||
await ctx.close();
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -113,6 +113,15 @@ func (l *Library) save() error {
|
||||
sort.Strings(a.MCP)
|
||||
sort.Strings(a.Skills)
|
||||
}
|
||||
// a skill or server on no agent is on none, [] not null: a list sorted
|
||||
// with slices.Sorted, or cloned, from an empty one is nil (#1217)
|
||||
for _, s := range l.Skills {
|
||||
s.Agents = orNone(s.Agents)
|
||||
}
|
||||
for _, s := range l.MCP {
|
||||
s.Agents = orNone(s.Agents)
|
||||
}
|
||||
l.Instructions.Agents = orNone(l.Instructions.Agents)
|
||||
b, err := json.MarshalIndent(l, "", " ")
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
@@ -208,7 +208,9 @@ func findNew(l *Library, repo, ref string, f *fetcher, limited *atomic.Pointer[e
|
||||
if err != nil {
|
||||
return nil, false
|
||||
}
|
||||
var agents []string
|
||||
// none when the repository's skills are on no agent: [], which the
|
||||
// page reads as a list (#1217)
|
||||
agents := []string{}
|
||||
for _, s := range skills {
|
||||
for _, a := range s.Agents {
|
||||
if !slices.Contains(agents, a) {
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"os"
|
||||
"net/http/httptest"
|
||||
"slices"
|
||||
"sort"
|
||||
@@ -217,3 +218,54 @@ func TestNewBase(t *testing.T) {
|
||||
t.Errorf("skills in the tree: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// char1eslu's report (#1217): skills from majiayu000/claude-arsenal and
|
||||
// two MCP servers, each switched off for every agent, were saved with
|
||||
// "agents": null; a check then offered the repository's other skills with
|
||||
// agents null, and the page threw drawing them. Every list of agents is
|
||||
// [] when it is empty: in library.json, and in what the page is sent.
|
||||
func TestNoAgentsIsEmptyNotNull(t *testing.T) {
|
||||
sandbox(t)
|
||||
fakeSkillRepo(t, map[string]string{
|
||||
"skills/codex-fluent/SKILL.md": "---\nname: codex-fluent\ndescription: Fluent\n---\n",
|
||||
"skills/codex-retrospective/SKILL.md": "---\nname: codex-retrospective\ndescription: Retro\n---\n",
|
||||
"skills/skill-usage-stats/SKILL.md": "---\nname: skill-usage-stats\ndescription: Stats\n---\n",
|
||||
})
|
||||
ok(t)(InstallSkills("majiayu000/claude-arsenal", []string{"skills/codex-fluent", "skills/codex-retrospective"}, []string{"claude"}))
|
||||
ok(t)(SkillAgents("codex-fluent", []string{}))
|
||||
ok(t)(SkillAgents("codex-retrospective", []string{}))
|
||||
ok(t)(SaveServer("", Server{Name: "stitch", Transport: "http", URL: "https://stitch.googleapis.com/mcp", Agents: []string{"gemini"}}))
|
||||
ok(t)(ServerAgents("stitch", []string{}))
|
||||
mu.Lock()
|
||||
l, _ := load()
|
||||
l.SeenSkills = nil // the third is new to the next check
|
||||
l.save()
|
||||
mu.Unlock()
|
||||
check(t)
|
||||
|
||||
b, err := os.ReadFile(path())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if strings.Contains(string(b), `"agents": null`) {
|
||||
t.Errorf("library.json has agents null:\n%s", b)
|
||||
}
|
||||
v, err := Read(nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(v.NewSkills) != 1 || v.NewSkills[0].Name != "skill-usage-stats" {
|
||||
t.Fatalf("new skills: %+v", v.NewSkills)
|
||||
}
|
||||
page, _ := json.Marshal(v)
|
||||
if strings.Contains(string(page), `"agents":null`) {
|
||||
t.Errorf("the page is sent agents null: %s", page)
|
||||
}
|
||||
|
||||
// added, it is on the agents the others are on: none, as []
|
||||
ok(t)(AddNewSkills([]string{v.NewSkills[0].ID}))
|
||||
b, _ = os.ReadFile(path())
|
||||
if !strings.Contains(string(b), `"skill-usage-stats"`) || strings.Contains(string(b), `"agents": null`) {
|
||||
t.Errorf("after adding the new one:\n%s", b)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user