webui: stop appending each skill card into its own checkbox

CHANGELOG.md

@@ -280,7 +280,6 @@ numbers follow the policy in [RELEASES.md](RELEASES.md).
   already the `--ok` token. `ui/app/core/layout.test.mjs` now checks every
   colour in the mark against the palette the sheet declares, the same guard
   that purged the borrowed palette from the code wells.
-
 - The web UI's Models view announces each panel's outcome on its own status
   line (`#models-status`, `#models-live-status`, `#models-catalog-status`)
   and writes failures too. One shared `aria-live` line was only ever written
@@ -289,6 +288,13 @@ numbers follow the policy in [RELEASES.md](RELEASES.md).
 
 ### Fixed
 
+- The web UI's Skills list (under Tools and Prompts) renders again when at
+  least one skill exists. The row-building callback in
+  `core/tools.js: loadSkills` shadowed the `#skills` container variable with
+  the row's own checkbox, so every card was appended into its checkbox — a
+  DOM hierarchy cycle the browser refuses — and the whole panel fell into its
+  error branch, showing "Could not load skills." with a Try again that reran
+  the same failure. Only an empty skills list ever displayed.
 - `clanker write-goal` (and the TUI `/write-goal` and the `goal_write` tool)
   no longer pastes the whole intent into every field its keywords cover: a
   rich intent used to come back with the objective, criterion, proof and

build.zig

@@ -293,6 +293,11 @@ pub fn build(b: *std.Build) void {
     const arena_js_test = b.addSystemCommand(&.{ "node", "--test" });
     arena_js_test.addFileArg(b.path("ui/app/features/arena.test.mjs"));
     test_step.dependOn(&arena_js_test.step);
+    // The Skills panel renders its cards into the container; the shipped
+    // loadSkills is driven over the DOM stub.
+    const skills_js_test = b.addSystemCommand(&.{ "node", "--test" });
+    skills_js_test.addFileArg(b.path("ui/app/core/skills.test.mjs"));
+    test_step.dependOn(&skills_js_test.step);
 
     // Logic that lives in a tool rather than in src/ still needs its tests run.
     // `zig build test` compiled only src/main.zig, so every `test` block under

docs/reports/README.md

@@ -191,6 +191,7 @@ Project agents receive this workflow through the harness prompt and
 ### Investigations
 
 <!-- inventory:investigation:start -->
+- [zig build test intermittently hangs forever in the bounded-chat abort test](investigations/2026-08-18-bounded-chat-abort-test-hangs.md) — Investigating
 - [Stale post-promotion checkout diff on doctor.zig was committed and pushed as revert 124d592e](investigations/2026-08-19-stale-checkout-diff-pushed-as-revert.md) — Investigating
 
 - [The REPL had no /rfc although clanker rfc exists](investigations/2026-08-17-missing-clanker-tool-rfc-slash-command-in-tui.md) — Resolved

docs/reports/investigations/2026-08-18-bounded-chat-abort-test-hangs.md

@@ -0,0 +1,40 @@
+# Investigation — zig build test intermittently hangs forever in the bounded-chat abort test
+
+## TL;DR
+
+- **Question:** llm.client's 'bounded chat aborts a provider that never sends a response' (client.zig:1624) intermittently never returns: the test binary sits at ~0% CPU indefinitely. Seen twice in four otherwise-green runs of one tree on 2026-08-18 (arm64 macOS), once for 3 hours. Suspect: the 2026-08-17 request watchdog sometimes fails to unblock the parked read. Recovery: kill the test binary (match by cwd) and rerun.
+- **Finding:** Investigating on 2026-08-18.
+- **Resolution:** Investigating on 2026-08-18.
+
+## Status
+
+Investigating on 2026-08-18.
+
+## Trigger and scope
+
+## Evidence
+
+## Hypotheses and tests
+
+## Finding
+
+## Resolution or handoff
+
+## References
+
+- Related bug: none yet
+## Evidence
+
+- 2026-08-18, run A: gate (zig build, zig build tools, zig build test) on 20fe5628 plus a webui-only diff. 'zig build test' stalled; the test binary showed 0.2% CPU after 30 minutes. 'sample <pid>' put the main thread in llm.client.test.bounded chat aborts a provider that never sends a response (client.zig:1624). Two other builds were running concurrently at the time.
+- 2026-08-18, run B: same command on a64bd7e2 plus a webui-only diff, no concurrent builds. Same test, same frame, ~0.1% CPU, still parked after 2 h 58 m. This rules out build concurrency as the trigger.
+- Two runs of the same tree(s) earlier the same day completed green (1623/1634 and 1625/1636, 11 skipped), so the hang is intermittent, roughly 2 in 4 today.
+- Three orphaned test binaries from another session's worktree, ~4.4 days old, were found parked in the sibling test cli.test.a provider that never answers costs the sweep its budget, not the OS connect timeout (+1556), all at 0% CPU. So both never-answering-provider tests can wedge, and a wedged one never ages out on its own.
+- The suite under test is the request watchdog shipped 2026-08-17 (agent.request_timeout_ms / stream_idle_timeout_ms; docs/reports/bugs/2026-08-17-agent-llm-call-has-no-deadline.md): the watchdog arms on a worker thread and Abort shutdown(2) is what unblocks the read. A hang here means that shutdown sometimes does not unblock the parked read, or the watchdog thread never fires.
+
+## Recovery
+
+- Identify the wedged binary by cwd, never by argv (the build runner's argv is relative): iterate pids from 'ps', match 'lsof -p <pid> -d cwd' against the worktree, confirm with 'sample <pid>' that the main thread is in one of the two never-answering-provider tests, then kill it and the parent 'zig build test'. A rerun of the same tree passes.
+- Reproduction is not on demand: the same tree passed twice and hung twice on the same day. Nothing here bisects to a change in the tree under test; the diffs in both hanging runs were webui-only files the test never touches.
+## Control run (2026-08-19)
+
+A pristine detached worktree of a64bd7e2 (origin/main, no diff at all) wedged the same way: 'zig build test' parked in llm.client.test.bounded chat aborts a provider that never sends a response at ~0% CPU, confirmed by 'sample' after the 10-minute threshold. That makes the score on a64bd7e2 four hangs in four runs (three on a webui-only diff, one on pristine main), against three completed green runs earlier the same day on 15c9138c/20fe5628 bases. The hang therefore lives on current main independent of any working diff, and became much more frequent (possibly deterministic) somewhere in 20fe5628..a64bd7e2 — af6703e2 ('toolhost registry, cli updates') and 317c257b/cecb96f8 are the commits in that window. Not bisected further; recovery remains kill-by-cwd and rerun, but on a64bd7e2 reruns no longer converge.

docs/reviews/webui.md

@@ -1441,6 +1441,39 @@ no longer has to stop at the module boundary. `serve --webui-port 41998`, then:
 Gate: `zig build`, `zig build tools`, `zig build test --summary all` — 163/163
 steps, 773/775 tests (2 skipped, the expected worktree pair).
 
+## The Skills list renders again (2026-08-18)
+
+The Skills panel under the tool rows has been dead for anyone with at least
+one file under `skills/`: it always showed "Could not load skills." with a
+Try again button that reran the identical failure. Only an empty skills list
+ever drew.
+
+The cause is one shadowed variable in `core/tools.js: loadSkills`. The
+function reads the container as `var box = document.getElementById("skills")`,
+and the per-skill callback then declared its checkbox as
+`var box = document.createElement("input")`. The closing
+`box.appendChild(card)` — meant to land the finished card in the container —
+appended it into its own checkbox instead. That is a hierarchy cycle (the
+checkbox is already a child of the card), which the DOM refuses with
+`HierarchyRequestError`; the throw landed in the fetch chain's `.catch`, whose
+message is the load-error one, so a rendering bug spent its whole life
+dressed as a network failure.
+
+### What changed
+
+- The checkbox is `check` now; `box` names only the container, and the card
+  lands in it. Nothing else moved.
+
+### Verified
+
+- `ui/app/core/skills.test.mjs` (new, wired into `zig build test`): drives the
+  **shipped** `loadSkills` source over `lib/dom-stub.mjs` with a stubbed
+  fetch. Control run first: against the unfixed code the non-empty case fails
+  (zero cards reach the container); with the fix, two skills render two
+  `.skill-card`s in `#skills`, each checkbox inside its card and empty, the
+  status line reads "2 skills.", and the error branch stays untouched. The
+  empty-list state still renders its own message.
+
 ## Each Models panel announces on its own status line (2026-08-18)
 
 `#models-status` was one `aria-live` line shared by three independent panels

ui/app/core/skills.test.mjs

@@ -0,0 +1,70 @@
+// Drives the shipped loadSkills over the DOM stub. The forEach callback used
+// to shadow the #skills container with the row's checkbox, so every card was
+// appended into its own checkbox (a hierarchy cycle the DOM refuses) and the
+// panel rendered "Could not load skills." whenever at least one skill existed.
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
+import test from "node:test";
+import "../lib/dom-stub.mjs";
+
+const here = dirname(fileURLToPath(import.meta.url));
+const src = readFileSync(join(here, "tools.js"), "utf8");
+
+// The real function text, run against the stub document and a stubbed fetch:
+// tools.js imports ui.js, whose vendor import node cannot resolve, so the
+// module is not importable here and the shipped source is extracted instead
+// (the harden.test.mjs pattern).
+function shippedLoadSkills(elements, fetchImpl) {
+  const m = /(function loadSkills\(\) \{[\s\S]*?\n\})/.exec(src);
+  assert.ok(m, "loadSkills missing from tools.js");
+  const doc = Object.create(globalThis.document);
+  doc.getElementById = function (id) { return elements[id] || null; };
+  const factory = new Function(
+    "document", "fetch", "_readJson", "utilFmtBytes", "showLoadError",
+    "return (" + m[1] + ");"
+  );
+  return factory(
+    doc,
+    fetchImpl,
+    function (x) { return x; },
+    function () { return "1 KB"; },
+    function () { elements.loadErrorShown = true; }
+  );
+}
+
+test("a non-empty skills list renders its cards into #skills", async function () {
+  const d = globalThis.document;
+  const els = { skills: d.createElement("div"), "skills-status": d.createElement("p") };
+  const loadSkills = shippedLoadSkills(els, function () {
+    return Promise.resolve({ skills: [
+      { name: "review.md", bytes: 1024, enabled: true, description: "how to review" },
+      { name: "ship.md", bytes: 2048, enabled: false }
+    ] });
+  });
+  await loadSkills();
+  assert.equal(els.loadErrorShown, undefined, "the happy path must not fall into the load-error branch");
+  const cards = els.skills.childNodes.filter(function (n) { return n.className === "skill-card"; });
+  assert.equal(cards.length, 2);
+  cards.forEach(function (card) {
+    assert.equal(card.parentNode, els.skills);
+    // The checkbox sits inside the card, never the other way around.
+    const check = card.childNodes[0];
+    assert.equal(check.tagName, "INPUT");
+    assert.equal(check.childNodes.length, 0);
+  });
+  assert.equal(els["skills-status"].textContent, "2 skills.");
+});
+
+test("an empty skills list still renders its empty state", async function () {
+  const d = globalThis.document;
+  const els = { skills: d.createElement("div"), "skills-status": d.createElement("p") };
+  const loadSkills = shippedLoadSkills(els, function () {
+    return Promise.resolve({ skills: [] });
+  });
+  await loadSkills();
+  assert.equal(els.loadErrorShown, undefined);
+  assert.match(els.skills.textContent, /No skills on file/);
+  assert.equal(els["skills-status"].textContent, "No skills.");
+});

ui/app/core/tools.js

@@ -411,26 +411,30 @@ function loadSkills() {
       list.forEach(function (sk) {
         var card = document.createElement("div");
         card.className = "skill-card";
-        var box = document.createElement("input");
-        box.type = "checkbox";
-        box.checked = sk.enabled !== false;
-        box.title = box.checked ? "Included in the system prompt" : "Off: not sent to the model";
-        box.addEventListener("change", function () {
-          box.disabled = true;
+        // Not named `box`: this callback used to shadow the #skills container
+        // with the checkbox, so the card was appended into its own checkbox,
+        // a hierarchy cycle the DOM refuses, which made any non-empty skills
+        // list render as "Could not load skills."
+        var check = document.createElement("input");
+        check.type = "checkbox";
+        check.checked = sk.enabled !== false;
+        check.title = check.checked ? "Included in the system prompt" : "Off: not sent to the model";
+        check.addEventListener("change", function () {
+          check.disabled = true;
           fetch("/api/skills", {
             method: "POST",
             headers: { "Content-Type": "application/json" },
-            body: JSON.stringify({ name: sk.name, enabled: box.checked })
+            body: JSON.stringify({ name: sk.name, enabled: check.checked })
           })
             .then(_readJson)
             .then(function () { return loadSkills(); })
             .catch(function (err) {
-              box.checked = !box.checked;
+              check.checked = !check.checked;
               if (status) status.textContent = "Skill: " + err.message;
             })
-            .then(function () { box.disabled = false; });
+            .then(function () { check.disabled = false; });
         });
-        card.appendChild(box);
+        card.appendChild(check);
         var name = document.createElement("span");
         name.className = "skill-name";
         name.textContent = sk.title || sk.name.replace(/\.md$/, "");