From 50fc924c435e86f087c4b86e595f62e836942033 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Thu, 22 Oct 2026 00:07:22 +0800 Subject: [PATCH 1/3] Fix edge cases in the triage sweeps A pull request force-pushed back to its base has no commit, and reading its check suites ended the PR sweep in a jq error. On a branch that still merges, a current hand-written rebase request was offered for hiding while the older fixed-text copy it superseded stayed. The issue sweep read only the last body edit, so a reporter who answered by editing stayed listed as waiting once a maintainer tidied the body; it also reported idle days for an assignee none of whose acts was read, missed a heading followed by a higher-level one or ending the body, took comments in a pasted shell session for headings, and ignored an uppercase hash. Test both sweeps against a fake gh in a new suite, and shellcheck the skills' scripts with the rest. --- .../skills/codetrial-triage/issue-sweep.sh | 50 +++- .claude/skills/codetrial-triage/pr-sweep.sh | 14 +- .claude/skills/codetrial-triage/pulls.md | 17 +- scripts/test.sh | 7 +- tests/test_skill_scripts.py | 281 ++++++++++++++++++ 5 files changed, 340 insertions(+), 29 deletions(-) create mode 100644 tests/test_skill_scripts.py diff --git a/.claude/skills/codetrial-triage/issue-sweep.sh b/.claude/skills/codetrial-triage/issue-sweep.sh index 91d0403d..c4acb58d 100755 --- a/.claude/skills/codetrial-triage/issue-sweep.sh +++ b/.claude/skills/codetrial-triage/issue-sweep.sh @@ -34,8 +34,8 @@ # is the newest comment; stale is null when the history is cut, # and idle_days too when none of their acts is in what was read # truncated issues with more than 100 comments or cross-references, 10 -# assignees or 20 assignments, whose lists above may miss what -# was cut +# assignees, 20 assignments or 20 body edits, whose lists above +# may miss what was cut set -eu @@ -48,8 +48,11 @@ query='query($owner: String!, $name: String!, $endCursor: String) { issues(states: OPEN, first: 50, after: $endCursor) { pageInfo { hasNextPage endCursor } nodes { - number title body createdAt lastEditedAt authorAssociation - author { login } editor { login } issueType { name } + number title body createdAt authorAssociation + author { login } issueType { name } + userContentEdits(last: 20) { + totalCount nodes { editedAt editor { login } } + } assignees(first: 10) { totalCount nodes { login } } assigned: timelineItems(last: 20, itemTypes: [ASSIGNED_EVENT]) { totalCount @@ -94,7 +97,9 @@ pages=$(gh api graphql --paginate -F owner="${repo%/*}" -F name="${repo#*/}" \ # collaborators with read or triage access, students among them, so the badge # counts only when the push listing could not be read. A bot's comment never # asks the reporter for anything. The reporter answering by editing the body -# counts, since the bug form and maintainers both ask for that. +# counts, since the bug form and maintainers both ask for that; every edit is +# read rather than only the last, which a maintainer tidying the body would +# otherwise hide. printf '%s\n' "$pages" | jq -s --argjson days "$days" --arg repo "$repo" \ --argjson pushers "$pushers" --arg me "$me" ' def ts: if . == null then 0 else fromdateiso8601 end; @@ -119,28 +124,48 @@ def ink: [limit(40; split("\n")[] | select(startswith("#") | not) [.[].data.repository.issues.nodes[]] as $issues | def maint: [.comments.nodes[] | select(human and staff)]; def cut: .comments.totalCount > 100 or .timelineItems.totalCount > 100 - or .assignees.totalCount > 10 or .assigned.totalCount > 20; + or .assignees.totalCount > 10 or .assigned.totalCount > 20 + or .userContentEdits.totalCount > 20; def prs: [.timelineItems.nodes[] | select(.source.number != null and .source.repository.nameWithOwner == $repo)]; def split: {open: [.[] | select(.state == "OPEN") | .number] | unique, merged: [.[] | select(.mergedAt != null) | .number] | unique}; def mine: .author.login as $a | [.comments.nodes[] | select($a != null and .author.login == $a) | .createdAt | ts] - + [if .editor.login == $a then .lastEditedAt | ts else 0 end] | max; + + [.userContentEdits.nodes[] | select($a != null and .editor.login == $a) + | .editedAt | ts] + [0] | max; +# Per line rather than one regex over the body, since "the same or a higher +# level" is not a backreference. A fence counts as content and what is inside +# it is skipped, so the comments in a pasted shell session are not headings; it +# closes only on its own character, at least as long, with nothing after it. +def empty_section: [foreach (split("\n")[] | select(test("\\S"))) as $l + ({fence: null, level: null}; + ([$l | capture("^ {0,3}(?`{3,}|~{3,})(?.*)$")] | first) as $f + | if .fence == null then + if $f != null and ($f.m[0:1] == "~" or ($f.rest | contains("`") | not)) + then {fence: $f.m, level: 0} + else .level = (($l | capture("^(?#{1,6}) +\\S") | .h | length) // 0) + end + elif $f != null and $f.m[0:1] == .fence[0:1] + and ($f.m | length) >= (.fence | length) and ($f.rest | test("^\\s*$")) + then {fence: null, level: 0} + else .level = null end; + .level | select(. != null))] as $lv + | any(range($lv | length); . as $k | $lv[$k] > 0 + and ($lv[$k + 1] // -1 | . == -1 or (. > 0 and . <= $lv[$k]))); def gaps: (.body // "") as $b | ($b + " " + ([.author.login as $a | .comments.nodes[] | select($a != null and .author.login == $a) | .body] | join(" "))) as $said | [ (select($b | ink < 40) | "empty_body"), (select($b | test("\\[([Pp]lease )?[Ff]ill in|<[A-Z][a-z][^<>\\n]{15,}>")) | "placeholder"), - (select($b | test("(?m)^(#{1,6}) [^\\n]+\\n\\s*(^\\1 |\\z)")) - | "empty_section"), + (select($b | empty_section) | "empty_section"), (select(.title | test("^\\s*(\\[[^\\]]+\\]|[A-Za-z]+(\\([^)]*\\))?:)\\s")) | "title_prefix"), (select((.title | cjk > latin) or (($b | cjk) > ($b | latin))) | "not_english"), (select(.issueType.name == "Bug" - and ($said | test("codetrial [0-9.]+ \\([0-9a-f]{7,40}\\)|\\b(?=[0-9a-f]*[0-9])(?=[0-9a-f]*[a-f])[0-9a-f]{7,40}\\b") | not)) + and ($said | test("codetrial [0-9.]+ \\([0-9a-f]{7,40}\\)|\\b(?=[0-9a-f]*[0-9])(?=[0-9a-f]*[a-f])[0-9a-f]{7,40}\\b"; "i") | not)) | "no_revision") ]; def claim: . as $i | maint as $m | (($pushers // []) + [$m[].author.login] @@ -150,8 +175,9 @@ def claim: . as $i | maint as $m | ([$i.assigned.nodes[] | select(.assignee.login == $a) | .createdAt] + [$i.comments.nodes[] | select(.author.login == $a) | .createdAt] + [$xrefs[].createdAt] | map(ts) | max) as $last - | ([$m[] | select((.createdAt | ts) > ($last // 0)) | .createdAt] - | max) as $check + | (if $last == null then null + else [$m[] | select((.createdAt | ts) > $last) | .createdAt] | max end) + as $check | (if $check then $check | ts else $last end) as $since | ([$i.comments.nodes[] | select(human)] | max_by(.createdAt | ts) | .author.login == $a) as $asked diff --git a/.claude/skills/codetrial-triage/pr-sweep.sh b/.claude/skills/codetrial-triage/pr-sweep.sh index edfa157e..90a0a3a7 100755 --- a/.claude/skills/codetrial-triage/pr-sweep.sh +++ b/.claude/skills/codetrial-triage/pr-sweep.sh @@ -62,7 +62,8 @@ known='["Rebase latest `main` and resolve conflicts", # first check suite, created when the push reached a workflow or app, or the # latest force push. The commit date, which the contributor's clock sets and # which can run ahead and make every later request look stale, is used only for -# a head with no check suite. +# a head with no check suite. A PR force-pushed back to its base has no commit +# at all, and dates only its force push. # shellcheck disable=SC2016 query='query($owner: String!, $name: String!, $endCursor: String) { repository(owner: $owner, name: $name) { @@ -131,7 +132,7 @@ printf '%s\n%s\n' "$prs" "$runs" | jq -s --argjson days "$days" --arg me "$me" \ --argjson pushers "$pushers" ' def human: .author != null and .author.__typename != "Bot"; def ts: if . == null then 0 else fromdateiso8601 end; -def pushed: .commits.nodes[0].commit as $c +def pushed: (.commits.nodes[0].commit // {checkSuites: {nodes: []}}) as $c | [([$c.checkSuites.nodes[].createdAt | ts] | min) // ($c.committedDate | ts), (.timelineItems.nodes[0].createdAt | ts)] | max; def trim: gsub("^\\s+|\\s+$"; ""); @@ -155,11 +156,10 @@ def requests: .author.login as $a | .headRefOid as $head | pushed as $p | def fixed_only: .inline == 0 and (.body | norm | IN($wordings[])); def asks_rebase: [requests[] | select(fixed_only)] | sort_by(.at | ts); def asked_here: any(requests[]; .current); -# A conflicting PR whose only request on its current head is hand-written: -# that request is the standing one, so no fixed one is kept beside it and it -# is never offered for hiding. -def hand_asked: .mergeable == "CONFLICTING" - and asked_here and all(asks_rebase[]; .current | not); +# A PR whose only request on its current head is hand-written, whether or not +# it still conflicts: that request is the standing one, so no fixed one is kept +# beside it and it is never offered for hiding. +def hand_asked: asked_here and all(asks_rebase[]; .current | not); def needs_rebase: .mergeable == "CONFLICTING" and (.isDraft | not) and .baseRefName == $base and $base == "main" and nudgeable and whole diff --git a/.claude/skills/codetrial-triage/pulls.md b/.claude/skills/codetrial-triage/pulls.md index 941959c9..c6b972e7 100644 --- a/.claude/skills/codetrial-triage/pulls.md +++ b/.claude/skills/codetrial-triage/pulls.md @@ -180,14 +180,15 @@ gh pr view "$pr" --repo "$repo" --json mergeable,headRefOid \ Post it before hiding anything, so a failed post never leaves a conflicting PR with no visible request. `hide` lists the earlier requests, reviews and comments alike, in a known wording and with no inline comments: all of them on a PR in -`rebase`, or on a conflicting PR whose current request is hand-written (that -request stays and is kept out of `hide_ask`); on any other PR, all but one: the -newest on the current head, or the newest overall when none is, since a review -left on an older head can carry a later date. The entries for a PR in `rebase` -assume its new request landed, so hide them only after that PR's post succeeded; -when the check above skipped the post, or the post failed, keep the one that -rule would keep, which is still the current request. Hiding a review does not -dismiss it; the PR stays at changes requested until a reviewer re-reviews. +`rebase`, or on a PR, conflicting or not, whose current request is hand-written +(that request stays and is kept out of `hide_ask`); on any other PR, all but +one: the newest on the current head, or the newest overall when none is, since a +review left on an older head can carry a later date. The entries for a PR in +`rebase` assume its new request landed, so hide them only after that PR's post +succeeded; when the check above skipped the post, or the post failed, keep the +one that rule would keep, which is still the current request. Hiding a review +does not dismiss it; the PR stays at changes requested until a reviewer +re-reviews. `hide_ask` lists the other requests that mention rebase on a PR that has, or is about to get, the fixed request: hand-written ones, and known-wording reviews diff --git a/scripts/test.sh b/scripts/test.sh index 34b77ce2..d92b95bc 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -316,13 +316,13 @@ shell_syntax() { status=0 scripts=0 - for script in "$ROOT"/scripts/*.sh; do + for script in "$ROOT"/scripts/*.sh "$ROOT"/.claude/skills/*/*.sh; do scripts=$((scripts + 1)) sh -n "$script" || status=1 done if command -v shellcheck > /dev/null 2>&1; then - (cd "$ROOT" && shellcheck scripts/*.sh) || status=1 + (cd "$ROOT" && shellcheck scripts/*.sh .claude/skills/*/*.sh) || status=1 else skip "shellcheck: installing it checks what $scripts shell scripts mean, not just that they parse" fi @@ -422,6 +422,9 @@ gate test-gate-tests unittest_gate "$ROOT/tests/test_test_gate.py" gate mutants-plan-tests unittest_gate "$ROOT/tests/test_mutants_plan.py" gate mutants-install-tests unittest_gate "$ROOT/tests/test_mutants_install.py" gate latency-tests unittest_gate "$ROOT/tests/test_analyze_latency.py" + +# The scripts the agent skills run are tested like any other, against a fake gh. +gate skill-scripts-tests unittest_gate "$ROOT/tests/test_skill_scripts.py" gate --lane browser browser-tests browser_tests static gate eslint eslint_gate gate cargo-audit cargo_audit_gate diff --git a/tests/test_skill_scripts.py b/tests/test_skill_scripts.py new file mode 100644 index 00000000..ac58a4f0 --- /dev/null +++ b/tests/test_skill_scripts.py @@ -0,0 +1,281 @@ +#!/usr/bin/env python3 +"""Contract tests for the scripts the agent skills under .claude/skills run. + +The sweeps read GitHub through `gh`, so each case serves them one GraphQL page +from a fake `gh` and asserts on the lists they print. +""" + +import importlib.util +import json +import os +import shutil +import subprocess +import tempfile +import unittest +from datetime import datetime, timedelta, timezone +from pathlib import Path + + +SPEC = importlib.util.spec_from_file_location( + "support", Path(__file__).with_name("support.py") +) +support = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(support) + +ROOT = support.ROOT +SKILLS = ROOT / ".claude" / "skills" +ISSUE_SWEEP = SKILLS / "codetrial-triage" / "issue-sweep.sh" +PR_SWEEP = SKILLS / "codetrial-triage" / "pr-sweep.sh" + +# The viewer and the one account with push access. The sweeps ask `gh` for +# both before the GraphQL page; the runs listing is empty, so no PR is held. +FAKE_GH = """#!/bin/sh +case "$*" in + "api user --jq .login") echo viewer ;; + *collaborators*) printf 'maint\\nviewer\\n' ;; + *actions/runs*) : ;; + "api graphql"*) cat "$FAKE_PAGE" ;; + *) echo "unexpected gh call: $*" >&2; exit 1 ;; +esac +""" + +# Enough prose and a pasted --version line that no lint fires unless a case +# asks for it. +FILLER = ( + "\n\ncodetrial 0.1.0 (abc1234), and enough words that the body is not " + "taken for an empty one." +) + + +def ago(days): + moment = datetime.now(timezone.utc) - timedelta(days=days) + return moment.strftime("%Y-%m-%dT%H:%M:%SZ") + + +def comment(login, days, body="Please post the `--version` line."): + return { + "author": {"__typename": "User", "login": login}, + "authorAssociation": "MEMBER" if login == "maint" else "NONE", + "createdAt": ago(days), + "body": body, + } + + +def issue(number, body, **fields): + node = { + "number": number, + "title": f"Interview stalls in case {number}", + "body": body, + "createdAt": ago(30), + "authorAssociation": "NONE", + "author": {"login": "reporter"}, + "issueType": {"name": "Bug"}, + "userContentEdits": {"totalCount": 0, "nodes": []}, + "assignees": {"totalCount": 0, "nodes": []}, + "assigned": {"totalCount": 0, "nodes": []}, + "comments": {"totalCount": 0, "nodes": []}, + "timelineItems": {"totalCount": 0, "nodes": []}, + } + return node | fields + + +def listed(*nodes): + return {"totalCount": len(nodes), "nodes": list(nodes)} + + +def pull(number, **fields): + node = { + "number": number, + "title": f"Fix case {number}", + "isDraft": False, + "authorAssociation": "CONTRIBUTOR", + "headRefOid": f"head{number}", + "mergeable": "MERGEABLE", + "baseRefName": "main", + "updatedAt": ago(1), + "headRefName": "topic", + "headRepository": {"nameWithOwner": "contributor/codetrial"}, + "author": {"login": "contributor"}, + "closingIssuesReferences": listed(), + "files": listed(), + # The head was pushed ten days ago, which dates every request. + "commits": { + "nodes": [ + { + "commit": { + "committedDate": ago(10), + "checkSuites": {"nodes": [{"createdAt": ago(10)}]}, + } + } + ] + }, + "reviews": listed(), + "comments": listed(), + "reviewThreads": listed(), + "timelineItems": {"nodes": []}, + } + return node | fields + + +def request(node_id, days, body): + return { + "id": node_id, + "author": {"__typename": "User", "login": "maint"}, + "authorAssociation": "MEMBER", + "createdAt": ago(days), + "isMinimized": False, + "body": body, + } + + +FIXED = "Rebase latest `main` branch and resolve conflicts." + + +@unittest.skipUnless(shutil.which("jq"), "jq is not installed") +class SweepCase(unittest.TestCase): + def sweep(self, script, page): + with tempfile.TemporaryDirectory() as scratch: + scratch = Path(scratch) + (scratch / "bin").mkdir() + support.write_executable(scratch / "bin" / "gh", FAKE_GH) + (scratch / "page.json").write_text(json.dumps({"data": page})) + result = subprocess.run( + ["sh", str(script), "sysprog21/codetrial"], + capture_output=True, + text=True, + env=os.environ + | { + "PATH": f"{scratch / 'bin'}:{os.environ['PATH']}", + "FAKE_PAGE": str(scratch / "page.json"), + }, + ) + self.assertEqual(result.returncode, 0, result.stderr) + return json.loads(result.stdout) + + +class IssueSweepTests(SweepCase): + def run_issues(self, *issues): + page = { + "repository": { + "issues": {"pageInfo": {"hasNextPage": False}, "nodes": list(issues)} + } + } + return self.sweep(ISSUE_SWEEP, page) + + def gaps(self, body): + lint = self.run_issues(issue(1, body))["lint"] + return lint[0]["gaps"] if lint else [] + + def test_heading_followed_by_a_higher_level_one_is_an_empty_section(self): + self.assertIn( + "empty_section", self.gaps("## Steps\n# Environment\nmacOS" + FILLER) + ) + + def test_heading_followed_by_its_own_subsection_is_not_empty(self): + self.assertEqual(self.gaps("## Steps\n### First\nOpen the lobby." + FILLER), []) + + def test_comments_in_a_fenced_shell_session_are_not_headings(self): + body = "## Steps\n```sh\n# start it\n# then\n```\nOpen the lobby." + self.assertEqual(self.gaps(body + FILLER), []) + + def test_a_shorter_fence_inside_a_longer_one_does_not_close_it(self): + body = "## Steps\n````md\n```\n# not a heading\n# nor this\n````\nOpen it." + self.assertEqual(self.gaps(body + FILLER), []) + + def test_heading_on_the_last_line_is_an_empty_section(self): + self.assertIn( + "empty_section", self.gaps("## Steps\nOpen it." + FILLER + "\n## Logs") + ) + + def test_uppercase_hash_counts_as_a_revision(self): + body = "Seen on build DEADBEEF1 after the reconnect, with enough prose here." + self.assertNotIn("no_revision", self.gaps(body)) + + def test_bug_without_a_revision_is_flagged(self): + body = "The interview stalls after reconnecting, every time, on two machines." + self.assertIn("no_revision", self.gaps(body)) + + def test_unanswered_question_is_waiting(self): + swept = self.run_issues( + issue(1, "Body" + FILLER, comments=listed(comment("maint", 10))) + ) + self.assertEqual([entry["issue"] for entry in swept["waiting"]], [1]) + + def test_reporter_edit_hidden_by_a_later_maintainer_edit_still_answers(self): + edits = listed( + {"editedAt": ago(2), "editor": {"login": "maint"}}, + {"editedAt": ago(5), "editor": {"login": "reporter"}}, + ) + swept = self.run_issues( + issue( + 1, + "Body" + FILLER, + comments=listed(comment("maint", 10)), + userContentEdits=edits, + ) + ) + self.assertEqual(swept["waiting"], []) + + def test_idle_days_is_unknown_when_no_act_of_the_assignee_was_read(self): + swept = self.run_issues( + issue( + 1, + "Body" + FILLER, + assignees=listed({"login": "student"}), + comments=listed(comment("maint", 10, "Assigned to you.")), + ) + ) + [claim] = swept["claimed"] + self.assertIsNone(claim["idle_days"]) + self.assertIsNone(claim["checked_at"]) + self.assertIsNone(claim["stale"]) + + def test_more_body_edits_than_read_marks_the_issue_truncated(self): + edits = {"totalCount": 21, "nodes": []} + swept = self.run_issues(issue(1, "Body" + FILLER, userContentEdits=edits)) + self.assertEqual(swept["truncated"], [1]) + + +class PrSweepTests(SweepCase): + def run_pulls(self, *pulls): + page = { + "repository": { + "defaultBranchRef": {"name": "main"}, + "pullRequests": { + "pageInfo": {"hasNextPage": False}, + "nodes": list(pulls), + }, + } + } + return self.sweep(PR_SWEEP, page) + + def test_pr_with_no_commits_does_not_end_the_sweep(self): + swept = self.run_pulls(pull(1, commits={"nodes": []}), pull(2)) + self.assertEqual([entry["number"] for entry in swept["prs"]], [1, 2]) + + def test_conflicting_pr_without_a_request_is_asked_to_rebase(self): + swept = self.run_pulls(pull(1, mergeable="CONFLICTING")) + self.assertEqual(swept["rebase"], [{"pr": 1, "author": "contributor"}]) + + def test_current_fixed_request_is_not_repeated_or_hidden(self): + swept = self.run_pulls( + pull(1, mergeable="CONFLICTING", comments=listed(request("IC_1", 5, FIXED))) + ) + self.assertEqual(swept["rebase"], []) + self.assertEqual(swept["hide"], []) + + def test_current_hand_written_request_supersedes_an_older_fixed_one(self): + comments = listed( + request("IC_old", 20, FIXED), + request("IC_new", 5, "Rebase latest `main` to pick up the zip fix."), + ) + for mergeable in ("MERGEABLE", "CONFLICTING"): + with self.subTest(mergeable=mergeable): + swept = self.run_pulls(pull(1, mergeable=mergeable, comments=comments)) + self.assertEqual([entry["id"] for entry in swept["hide"]], ["IC_old"]) + self.assertEqual(swept["hide_ask"], []) + self.assertEqual(swept["rebase"], []) + + +if __name__ == "__main__": + unittest.main() From 46754cea3609d34f3aa45c36e02a34b649937919 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Thu, 22 Oct 2026 00:07:22 +0800 Subject: [PATCH 2/3] Extend the skills for reviews, CI and UI work The skills said how to read a review but not how to fetch every inline thread, a long one included, or what a push leaves unproven. They said which lane to rerun but not how to tell a current failure from one on an older head. They named the UI rules reviewers raise but gave no walk through a changed flow, no Playwright method that reuses the existing browser checks, and no rules for applying a requested label, assignee, sub-issue or dependency. Add each where its owner already sits, linking the rules other sections hold rather than restating them, and state the existing rule on asking about unclear review items once. --- .claude/skills/codetrial-contribute/SKILL.md | 6 +- .../codetrial-contribute/review-feedback.md | 56 +++++++++++++++++++ .claude/skills/codetrial-triage/SKILL.md | 16 +++++- .claude/skills/codetrial-triage/waiting.md | 7 +-- .claude/skills/codetrial-verify/SKILL.md | 43 +++++++++++++- .claude/skills/codetrial-verify/review.md | 5 +- .claude/skills/codetrial-web/SKILL.md | 17 +++++- .claude/skills/codetrial-web/ui-review.md | 39 +++++++++++++ 8 files changed, 177 insertions(+), 12 deletions(-) create mode 100644 .claude/skills/codetrial-contribute/review-feedback.md create mode 100644 .claude/skills/codetrial-web/ui-review.md diff --git a/.claude/skills/codetrial-contribute/SKILL.md b/.claude/skills/codetrial-contribute/SKILL.md index 2c8f70b2..af1a7b03 100644 --- a/.claude/skills/codetrial-contribute/SKILL.md +++ b/.claude/skills/codetrial-contribute/SKILL.md @@ -1,6 +1,6 @@ --- name: codetrial-contribute -description: Help CodeTrial contributors turn observations into clear English GitHub issues, small contribution plans, and pull request descriptions, and see them through. Use to draft, improve or file a CodeTrial issue, answer what a maintainer asked on it (a revision, a log, an initial analysis, a retest), prepare a PR description, respond to review requests such as rebase, squash or commit message fixes, check which of your own issues and PRs are waiting on you, ask a focused maintainer question, or choose a bounded first contribution with an AI agent. The deliverable is copy-ready text; judging an existing backlog for duplicates is codetrial-triage. Opening a PR goes through gh-submit when it is installed; this skill covers it otherwise. +description: Help CodeTrial contributors turn observations into clear English GitHub issues, small contribution plans, and pull request descriptions, and see them through. Use to draft, improve or file a CodeTrial issue, answer what a maintainer asked on it (a revision, a log, an initial analysis, a retest), prepare a PR description, retrieve and address PR review comments, respond to requests such as rebase, squash or commit message fixes, check which of your own issues and PRs are waiting on you, ask a focused maintainer question, or choose a bounded first contribution with an AI agent. The deliverable is copy-ready text, or verified fixes when asked to address review feedback; judging an existing backlog for duplicates is codetrial-triage. Opening a PR goes through gh-submit when it is installed; this skill covers it otherwise. --- # Contribute to CodeTrial @@ -235,6 +235,10 @@ everyone's list. ## Answer a review +Fetching every thread, including long ones, and rechecking the head after a +push, and where failed checks are diagnosed, are in +[review-feedback.md](review-feedback.md). + Git and commit-message requests recur alongside substantive code review. Read the current inline threads, unanswered cubic findings and failing checks as well as the review summary. The one-line requests are decoded in diff --git a/.claude/skills/codetrial-contribute/review-feedback.md b/.claude/skills/codetrial-contribute/review-feedback.md new file mode 100644 index 00000000..122b87b9 --- /dev/null +++ b/.claude/skills/codetrial-contribute/review-feedback.md @@ -0,0 +1,56 @@ +# Retrieve and close out review feedback + +What a request means, how to answer it and the local list of open requests are +in [Answer a review](SKILL.md#answer-a-review); this file only covers fetching +every thread and confirming the result. A request only to review or summarize a +PR stays read-only. When the user names a subset of comments, address that +subset; "address all the feedback" is the whole list, not a prompt to ask which. + +Run `gh auth status` first, so an access failure is reported as unreviewed +feedback rather than as a PR with none. Top-level comments and reviews come from +`gh pr view`; inline threads need their own query, with the ids and URLs that +replying and resolving need: + +```sh +gh pr view "$pr" --repo sysprog21/codetrial \ + --json number,url,headRefOid,comments,reviews +gh api graphql --paginate -F n="$pr" -f query='query($n: Int!, $endCursor: String) { + repository(owner: "sysprog21", name: "codetrial") { pullRequest(number: $n) { + reviewThreads(first: 100, after: $endCursor) { + pageInfo { hasNextPage endCursor } + nodes { id isResolved isOutdated path line originalLine + comments(first: 100) { + pageInfo { hasNextPage } + nodes { id author { login } body url } } } } } } }' +``` + +`--paginate` follows only the outer `pageInfo` and prints one JSON document per +page. A thread whose `comments.pageInfo.hasNextPage` is true is incomplete; +refetch all of its comments, where the thread's own `pageInfo` is the only one, +before reading its last reply: + +```sh +gh api graphql --paginate -F thread="$thread" -f query='query($thread: ID!, $endCursor: String) { + node(id: $thread) { ... on PullRequestReviewThread { + comments(first: 100, after: $endCursor) { + pageInfo { hasNextPage endCursor } + nodes { id author { login } body url } } } } }' +``` +An outdated position does not mean fixed; compare the request with the current +code. Verify a bot finding by the same standard as any other +([collaboration.md](collaboration.md#review-evidence-including-bot-findings)). + +Fix each request with the smallest change and run the affected check, then the +full gate with its skips read; browser behavior goes through +[Browser investigation](../codetrial-verify/SKILL.md#browser-investigation) +and red checks through +[Failed GitHub Actions checks](../codetrial-verify/SKILL.md#failed-github-actions-checks). +If the scope changed, update the PR description to describe the final result. + +A push does not prove every request addressed. After it, rerun both queries +above and the checks, so a new top-level review or comment is read as well as +a new thread, and reassess anything a new head or new comment touches. +Replies and resolving follow +[codetrial-conventions](../codetrial-conventions/SKILL.md#pull-request-branches-and-review-replies) +and `CONTRIBUTING.md`, which closes an addressed thread with "Resolve +conversation"; never resolve a disputed or unverified request. diff --git a/.claude/skills/codetrial-triage/SKILL.md b/.claude/skills/codetrial-triage/SKILL.md index 8452bcb9..64004215 100644 --- a/.claude/skills/codetrial-triage/SKILL.md +++ b/.claude/skills/codetrial-triage/SKILL.md @@ -6,8 +6,9 @@ description: > review, and draft the reminders, acceptances and closes that follow. Use for backlog triage, checking new issues or PRs, asking what needs attention, deciding whether a symptom already has an issue, explicit PR sweeps, - approving GitHub Actions runs held for first-time contributors, and asking - conflicting PRs to rebase. Default to a read-only report; a PR sweep has + approving GitHub Actions runs held for first-time contributors, asking + conflicting PRs to rebase, and applying requested label, assignee, issue type, + sub-issue or blocked-by changes. Default to a read-only report; a PR sweep has narrowly defined actions in pulls.md. Use codetrial-contribute for drafts on a contributor's behalf, new issues or PRs, and focused maintainer questions. --- @@ -403,6 +404,17 @@ that error, comment `Duplicate of #N` and close with `--reason "not planned"`, which on its own reads as rejected and would take a reopen to correct, notifying the reporter a second time for nothing. +A sub-issue, a blocked-by dependency and a duplicate are three different +records, and a cross-link in prose establishes none of them. `gh issue edit` +covers labels and assignees; for issue types, sub-issues and dependencies read +GitHub's current API documentation for that operation rather than guessing an +endpoint, check which issue is the parent or the blocker, and pass a node or +database ID where the endpoint wants one instead of an issue number. Put the +request body in a JSON file and send it with +`gh api --method --input "$SCRATCH/payload.json"`, then read +the relationship back. If the operation is unavailable, say so and list the +intended relationship as a proposal, never as applied. + Post any preserved reproduction evidence to the canonical issue before closing. Do not pair `--duplicate-of` with a `Duplicate of #N` comment; GitHub's marker already says it. Report applied actions separately from proposals, with links. diff --git a/.claude/skills/codetrial-triage/waiting.md b/.claude/skills/codetrial-triage/waiting.md index 57a0941b..288421e4 100644 --- a/.claude/skills/codetrial-triage/waiting.md +++ b/.claude/skills/codetrial-triage/waiting.md @@ -38,10 +38,9 @@ The four sweep lists are the queue, and each one has a fixed next move: disposition first. When a linked PR merges or the facts change while a reminder is pending, rescope the request and keep its deadline. The closing comment names the fact again and says a maintainer reopens the issue once it -is posted, since a comment alone does not reopen it and the sweep reads only -open issues. An - issue a maintainer opened is left out of `waiting`, because its author already - knows the rule. + is posted, since a comment alone does not reopen it and the sweep reads only + open issues. An issue a maintainer opened is left out of `waiting`, because + its author already knows the rule. - `linked`, with `merged` set: a merged pull request referenced the issue after it was filed, and GitHub did not close it, so that PR only touched it, or its `Closes` line named another issue. Read the PR and the issue. When the PR does diff --git a/.claude/skills/codetrial-verify/SKILL.md b/.claude/skills/codetrial-verify/SKILL.md index fbab0546..a91dc515 100644 --- a/.claude/skills/codetrial-verify/SKILL.md +++ b/.claude/skills/codetrial-verify/SKILL.md @@ -1,6 +1,6 @@ --- name: codetrial-verify -description: How a CodeTrial change is validated - scripts/test.sh as the credential-free gate, which generated artifacts have to be regenerated before it passes, the checks that need credentials or a browser and therefore sit outside it, the browser and mutation lanes, and how to bring the server up for a live look without taking the maintainer's port. Use before calling work done, when a gate fails on drift rather than on a bug, when adding a test, when a fix needs to be seen working in the app, when reviewing a diff or an open pull request against the contracts in docs/ (review.md), or after editing a skill or anything a skill names (skill-refs.sh). +description: How a CodeTrial change is validated - scripts/test.sh as the credential-free gate, which generated artifacts have to be regenerated before it passes, the checks that need credentials or a browser and therefore sit outside it, the browser and mutation lanes, and how to bring the server up for a live look without taking the maintainer's port. Use before calling work done, when diagnosing failed GitHub Actions checks, when a gate fails on drift rather than on a bug, when adding a test, when a fix needs to be seen working in the app, when reviewing a diff or an open pull request against the contracts in docs/ (review.md), or after editing a skill or anything a skill names (skill-refs.sh). --- # Validating a CodeTrial change @@ -112,6 +112,45 @@ is worth and `mutants` runs `cargo-mutants` over it. A surviving mutant means a line changed behavior with no test noticing, so the answer is a test, not a retry. +## Browser investigation + +Drive pages with the repository's Node Playwright dependency; no Python +Playwright environment and no second server wrapper. `scripts/browser-check.sh`, +whose validation block lists its `BROWSER_CHECK_FLOW` values, and +`scripts/visual-parity-check.sh` pick a free port and clean up after themselves; +for an exploratory session use the separate-port recipe in +[Seeing it work in the app](#seeing-it-work-in-the-app) and stop only the +server you started. LiveKit pages never go network-idle, so wait for the +specific control or state the action needs. Attach console and page-error +listeners before navigating, keep screenshots in `$SCRATCH` with credentials and +interview content removed, and look at a screenshot before claiming a visual +result. Never refresh `tests/golden/visual` merely to silence a difference; +establish that the new output is intended. A behavior change gets its +regression check in the existing lane. + +## Failed GitHub Actions checks + +The lanes and the aggregate `check` and `mutation` jobs are described above; +telling a stale base, a regression, a race and an environment limit apart is in +[collaboration.md](../codetrial-contribute/collaboration.md#make-ci-green-before-asking-for-review); +reading the job log, and why it is `--log`, is the recipe in +[pulls.md](../codetrial-triage/pulls.md#recheck-dependencies-and-substantive-review). +What this adds is checking that the failure is current: + +```sh +gh pr view "$pr" --repo sysprog21/codetrial --json headRefOid +gh pr checks "$pr" --repo sysprog21/codetrial --json name,state,bucket,link +gh run view "$run" --repo sysprog21/codetrial --json headSha,status,conclusion,jobs +``` + +A run whose `headSha` is not the PR head is history, not the current failure. +`gh pr checks` exits non-zero when a check fails and still prints its rows. An +empty list after `gh auth status` fails is an access gap; a held run has no log +yet. A third-party check is reported by name and details URL. Reproduce with the +lane at the same head, fix the cause, run the lane and then the full gate, and +after an authorized push reread the head and checks: pending stays pending in +the report. + ## The git hooks `make hooks` installs the fast half of the gate at commit time: @@ -201,7 +240,7 @@ rebase or amend: | --- | --- | --- | | fixed | the original reproduction now behaves | the code changed | | tests pass | `./scripts/test.sh` at this head, its skips read | an earlier run | -| CI is green | `gh pr checks` naming this head | a local pass | +| CI is green | `gh pr checks` passing, and each run's `headSha` from `gh run view` at this head | a local pass, or a run on an older head | | review addressed | each open thread reread at this head | a pushed commit | | the agent did it | `git diff` shows the change | the agent's summary | diff --git a/.claude/skills/codetrial-verify/review.md b/.claude/skills/codetrial-verify/review.md index 0d0f2abf..f57a2db0 100644 --- a/.claude/skills/codetrial-verify/review.md +++ b/.claude/skills/codetrial-verify/review.md @@ -19,8 +19,9 @@ gh pr diff "$pr" --repo sysprog21/codetrial > "$SCRATCH/pr-$pr.diff" Group the changed files by the area they touch, an area being a row of the table in step 2 or "other", and review one area at a time; past about 500 -changed lines, report per area. Read the PR's linked issue and the maintainer -comment that accepted its scope +changed lines, report per area. A wire change is one area however its files +split: review the `web/` and Rust sides of a topic together. Read the PR's +linked issue and the maintainer comment that accepted its scope ([collaboration.md](../codetrial-contribute/collaboration.md#start-with-an-actionable-contribution)): that comment is the specification, and work outside it is a finding. When the issue was only assigned, its body's scope stands in, and that is worth saying. diff --git a/.claude/skills/codetrial-web/SKILL.md b/.claude/skills/codetrial-web/SKILL.md index 8bba10ff..4bac9fc7 100644 --- a/.claude/skills/codetrial-web/SKILL.md +++ b/.claude/skills/codetrial-web/SKILL.md @@ -1,6 +1,6 @@ --- name: codetrial-web -description: The CodeTrial browser half and the boundary it talks across - why web/ has no build step, how a file reaches the browser embedded or from disk, the vendored checksum-pinned assets, the data-channel topics implemented once in web/lib.js and once against the TOPIC_ constants in src/runtime.rs, in both directions, and the wire fixtures that keep the two agreeing. Use when editing anything under web/, adding or changing a data-channel message, touching src/web/, or wondering why a browser change passed every Rust test and still broke the interview. +description: The CodeTrial browser half and the boundary it talks across - why web/ has no build step, how a file reaches the browser embedded or from disk, the vendored checksum-pinned assets, the data-channel topics implemented once in web/lib.js and once against the TOPIC_ constants in src/runtime.rs, in both directions, and the wire fixtures that keep the two agreeing. Use when editing anything under web/, adding or changing a data-channel message, touching src/web/, reviewing UI accessibility or interaction states, designing a new page or an explicit redesign, or wondering why a browser change passed every Rust test and still broke the interview. --- # The CodeTrial browser boundary @@ -101,6 +101,21 @@ The gate cannot see these, and reviews here keep raising them: modifier chords it does not own; the Cmd/Ctrl+Enter handler in `web/interview.js` has all three guards (#144, #277). +## UI behavior and design + +For an interface review, walk the changed flow through the checks in +[ui-review.md](ui-review.md), then verify the behavior with the existing browser +lanes described in [codetrial-verify](../codetrial-verify/SKILL.md#browser-investigation), +and report what could not run apart from what failed. + +For a new page or an explicitly requested redesign, first name the user's task, +the visual hierarchy and the layout at narrow and wide widths, and read the +neighboring pages. The tokens and themes above still apply. A visual brief does +not call for a framework, a build step, an external font download or a restyle +of unrelated pages. Review rendered screenshots for hierarchy, readability and +overflow, then exercise the interactions; a screenshot alone cannot validate +them. + ## Generated pages `web/problems/`, `web/judges/` and the problem cards inside `web/index.html` diff --git a/.claude/skills/codetrial-web/ui-review.md b/.claude/skills/codetrial-web/ui-review.md new file mode 100644 index 00000000..2cfffaee --- /dev/null +++ b/.claude/skills/codetrial-web/ui-review.md @@ -0,0 +1,39 @@ +# Review the changed user flow + +The accessibility, theme and key-handling rules reviews keep raising are in +[What review checks in a UI change](SKILL.md#what-review-checks-in-a-ui-change); +this file adds the walk through a flow. Check only the controls and states the +change touches. A finding is evidence for a proposed fix, not permission to +redesign the page. + +- Keyboard: Tab and Shift+Tab reach every control, a composite one such as a + radio group as a single stop whose items the arrow keys move between, and + each activates with its native keys; hidden or inert content takes no focus. + The editor splitter's drag has a keyboard alternative. +- Semantics: actions are buttons and navigation is links. Icon buttons have an + accessible name and inputs a real label, not only a placeholder. Decorative + icons are hidden from assistive technology. +- Forms: input accepts paste and submits by keyboard. A failed submission keeps + what was entered, ties the error to its field and moves focus to the first + invalid one. +- Async states: walk idle, loading, success, empty and error, and the retry after + an error. Pending work shows feedback and blocks a duplicate submission; a + status change is announced without taking focus; an empty result does not look + like an unfinished load; error text names a next step. +- Layout: narrow widths, zoom, long labels and reduced motion. + +The CodeTrial transitions to walk when the change touches them: lobby account +and history loading, microphone and camera preflight, entering the interview, +reconnecting, ending the session, and showing or deleting a saved report. A +pending login or Start keeps the controls until it completes, and an error +releases them for a retry without losing the chosen problem, language or +interview settings. During an interview, editor shortcuts, captions and media +controls stay usable together. Deleting a report moves focus to the row that +took its place, or to the status line when none remain, and announces the +result. + +Inspect the rendered DOM and accessibility tree alongside screenshots, with +synthetic interview content. For each failure record the route, viewport and +theme, action and observed result so a browser check can reproduce it; keeping +screenshots and console output is in +[Browser investigation](../codetrial-verify/SKILL.md#browser-investigation). From 6a251dfb6885c81f0b658568eca79ee92a78b39c Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Thu, 22 Oct 2026 00:07:22 +0800 Subject: [PATCH 3/3] Check skill references against GitHub anchors The reference checker was awk over the skills, outside the gate, and guessed wrong in both directions. It gave a repeated heading, closing hashes, a link in a heading and indented or underlined headings anchors GitHub does not, read headings inside tilde or longer fences, checked links in inline code, and cut make targets at an underscore or a digit, while a link title, a path:line reference or a flag that is a prefix of another raised false alarms. Rewrite it as scripts/check-skill-refs.py, which follows GitHub's slug rule and parses fences and code spans once, also checks code blocks and root files, and runs as a gate with tests against a scratch repository. --- .claude/skills/codetrial-verify/SKILL.md | 18 +- .claude/skills/codetrial-verify/review.md | 2 +- .claude/skills/codetrial-verify/skill-refs.sh | 68 ----- scripts/check-skill-refs.py | 260 ++++++++++++++++++ scripts/test.sh | 5 +- tests/test_skill_scripts.py | 132 ++++++++- 6 files changed, 405 insertions(+), 80 deletions(-) delete mode 100755 .claude/skills/codetrial-verify/skill-refs.sh create mode 100755 scripts/check-skill-refs.py diff --git a/.claude/skills/codetrial-verify/SKILL.md b/.claude/skills/codetrial-verify/SKILL.md index a91dc515..396c8447 100644 --- a/.claude/skills/codetrial-verify/SKILL.md +++ b/.claude/skills/codetrial-verify/SKILL.md @@ -1,6 +1,6 @@ --- name: codetrial-verify -description: How a CodeTrial change is validated - scripts/test.sh as the credential-free gate, which generated artifacts have to be regenerated before it passes, the checks that need credentials or a browser and therefore sit outside it, the browser and mutation lanes, and how to bring the server up for a live look without taking the maintainer's port. Use before calling work done, when diagnosing failed GitHub Actions checks, when a gate fails on drift rather than on a bug, when adding a test, when a fix needs to be seen working in the app, when reviewing a diff or an open pull request against the contracts in docs/ (review.md), or after editing a skill or anything a skill names (skill-refs.sh). +description: How a CodeTrial change is validated - scripts/test.sh as the credential-free gate, which generated artifacts have to be regenerated before it passes, the checks that need credentials or a browser and therefore sit outside it, the browser and mutation lanes, and how to bring the server up for a live look without taking the maintainer's port. Use before calling work done, when diagnosing failed GitHub Actions checks, when a gate fails on drift rather than on a bug, when adding a test, when a fix needs to be seen working in the app, when reviewing a diff or an open pull request against the contracts in docs/ (review.md), or after editing a skill or anything a skill names (scripts/check-skill-refs.py). --- # Validating a CodeTrial change @@ -27,8 +27,8 @@ Reproduce a failure with the same lane locally. one failure does not hide the next; the summary line names every gate that failed. It covers `cargo fmt --check`, `clippy -D warnings`, `cargo test`, the Python unittest suites, the Node browser tests, ESLint, `ruff check`, -`shellcheck`, the generated-artifact drift checks, the hook suite, `cargo-audit` -and `actionlint`. +`shellcheck`, the generated-artifact drift checks, the hook suite, the skill +references, `cargo-audit` and `actionlint`. It is not offline. The `fetch-vendor` gate downloads missing assets named by a `web/vendor/**/FETCH` manifest; it cannot restore committed vendor files. @@ -253,9 +253,9 @@ A passing gate is not a review; reviewing a diff, one's own or an open pull request, follows [review.md](review.md). A skill names gates, lanes, scripts, flags, make targets and documents, and -each one can move. After changing either side, run -`.claude/skills/codetrial-verify/skill-refs.sh`: it prints every reference a -skill makes that no longer resolves, and every check script this skill does -not list. It catches a -rename, not a change of meaning, so a changed rule still needs the skill -reread. +each one can move. The `skill-refs` gate runs `scripts/check-skill-refs.py`, +which prints every reference a skill makes that no longer resolves and every +check script this skill does not list; run it on its own after changing either +side. It catches a rename, not a change of meaning, so a changed rule still +needs the skill reread. The scripts the skills run are tested in +`tests/test_skill_scripts.py`. diff --git a/.claude/skills/codetrial-verify/review.md b/.claude/skills/codetrial-verify/review.md index f57a2db0..a199a488 100644 --- a/.claude/skills/codetrial-verify/review.md +++ b/.claude/skills/codetrial-verify/review.md @@ -109,7 +109,7 @@ report what it reports. For each area, look for: version bump that frees it. - Skills. A diff that renames or removes a gate, a lane, a CI job, a script, a flag or a make target, or changes a maintainer wording, leaves stale the - skill that names it; run [skill-refs.sh](skill-refs.sh) and reread the skills + skill that names it; run `scripts/check-skill-refs.py` and reread the skills that name it. A new file moves nothing. Done when each area has been read against every item above, with a finding or diff --git a/.claude/skills/codetrial-verify/skill-refs.sh b/.claude/skills/codetrial-verify/skill-refs.sh deleted file mode 100755 index a8b22c57..00000000 --- a/.claude/skills/codetrial-verify/skill-refs.sh +++ /dev/null @@ -1,68 +0,0 @@ -#!/bin/sh -# Report every reference in the skills that no longer resolves: relative links -# and their anchors, repo paths in backticks, make targets and CI lane names. -# One line per dangling reference; exits 1 when there is any. -set -u -cd "$(git rev-parse --show-toplevel)" || exit 2 - -# GitHub's heading anchor: lowercase, punctuation dropped, spaces to hyphens. -slugs() -{ - awk '/^```/ { f = !f; next } !f && sub(/^#+ /, "")' "$1" | tr '[:upper:]' '[:lower:]' \ - | sed 's/[^a-z0-9 _-]//g; s/ /-/g' -} - -check() -{ - md=$1 - dir=$(dirname "$md") - grep -oE '\]\([^):]+\)' "$md" | sed 's/^](//; s/)$//' \ - | while IFS= read -r link; do - file=${link%%#*} - target=$md - [ -n "$file" ] && target=$dir/$file - if [ ! -e "$target" ]; then - echo "$md: link $link" - elif [ "$file" != "$link" ] && ! slugs "$target" | grep -qxF "${link#*#}"; then - echo "$md: anchor $link" - fi - done - # shellcheck disable=SC2016 - grep -oE '`(\./)?(scripts|docs|src|web|tests|problem-bank|\.github|\.claude)/[^` ]*' "$md" \ - | sed 's/^`//; s|^\./||' | grep -v '[*<>{}$]' | sort -u \ - | while IFS= read -r path; do - [ -e "$path" ] || echo "$md: path $path" - done - # shellcheck disable=SC2016 - grep -oE '`make [a-z-]+' "$md" | cut -c7- | sort -u \ - | while IFS= read -r target; do - grep -q "^$target:" Makefile || echo "$md: make $target" - done - # shellcheck disable=SC2016 - grep -oE '`(\./)?scripts/[a-z0-9_.-]+ --[a-z-]+' "$md" | tr -d '`' \ - | sed 's|^\./||' | sort -u \ - | while read -r script flag; do - grep -qF -- "$flag" "$script" 2> /dev/null \ - || echo "$md: flag $script $flag" - done - grep -oE 'gates \([a-z]+\)' "$md" | tr -d '()' | cut -d' ' -f2 | sort -u \ - | while IFS= read -r lane; do - grep -qE "^ *- lane: $lane\$" .github/workflows/check.yml \ - || echo "$md: CI lane $lane" - done -} - -# The other direction: a check script outside the gate that no skill names. -unlisted() -{ - for script in scripts/*-check.sh; do - grep -qF "/$script" scripts/test.sh && continue - grep -qF "$script" .claude/skills/codetrial-verify/SKILL.md \ - || echo "codetrial-verify/SKILL.md: does not name $script" - done -} - -! { - for md in .claude/skills/*/*.md; do check "$md"; done - unlisted -} | grep . diff --git a/scripts/check-skill-refs.py b/scripts/check-skill-refs.py new file mode 100755 index 00000000..083339b3 --- /dev/null +++ b/scripts/check-skill-refs.py @@ -0,0 +1,260 @@ +#!/usr/bin/env python3 +"""Report every reference in the skills that no longer resolves. + +The skills under `.claude/skills/` name relative links and their anchors, repo +paths and root files in backticks or code blocks, make targets, script flags and +CI lane names, and each one can move. This prints one line per dangling +reference, plus every check script outside the gate that codetrial-verify does +not name, and exits 1 when there is any. It catches a rename, not a change of +meaning. + +Anchors follow GitHub's rule rather than a guess at it, because a link whose +anchor is wrong still opens the file and an agent reading it never notices. +""" + +import functools +import glob +import os +import re +import sys +import unicodedata + +ROOTS = r"(?:scripts|docs|src|web|tests|problem-bank|\.github|\.claude)" +# A root file named bare in prose; these extensions are the ones the skills +# name, so a bare `lib.js` is not taken for a root file. +ROOT_FILE = re.compile( + r"[A-Za-z0-9_.-]+\.(?:md|toml|json|mjs|cjs|yml|yaml|lock)|Makefile" +) +# Untracked and per-developer by design, so a clone may lack them. +UNTRACKED = {"TODO.md", "DONE.md", "CLAUDE.md"} +FENCE = re.compile(r"^\s*(`{3,}|~{3,})(.*)$") +TICKS = re.compile(r"`+") + + +def blocks(text): + """Yield (in_fence, line) with front matter dropped; a fence closes only on + its own character, at least as long, with nothing after it.""" + lines = text.split("\n") + if lines and lines[0] == "---": + end = next((i for i in range(1, len(lines)) if lines[i] == "---"), 0) + lines = lines[end + 1 :] + fence = None + for line in lines: + m = FENCE.match(line) + if fence is None and m and not (m.group(1)[0] == "`" and "`" in m.group(2)): + fence = m.group(1) + yield True, None + continue + if fence is not None: + if ( + m + and m.group(1)[0] == fence[0] + and len(m.group(1)) >= len(fence) + and not m.group(2).strip() + ): + fence = None + else: + yield True, line + continue + yield False, line + + +def code_spans(line): + """Split a line into prose and inline code: a run of n backticks closes + only at the next run of exactly n.""" + prose, code, i = [], [], 0 + while i < len(line): + m = TICKS.search(line, i) + if not m: + prose.append(line[i:]) + break + prose.append(line[i : m.start()]) + close = re.compile(r"(?` and `__init__` included.""" + + def markup(prose): + prose = re.sub(r"!?\[([^\]]*)\]\([^)]*\)", r"\1", prose) + prose = re.sub(r"<[^>]+>", "", prose) + return re.sub(r"(\*+|(?|]|\d+[.)]|#)", line) + and heading is None + else None + ) + if heading is None: + continue + base = slug(heading) + n = seen.get(base, 0) + seen[base] = n + 1 + found.add(base if n == 0 else f"{base}-{n}") + return found + + +def make_targets(): + targets = set() + with open("Makefile", encoding="utf-8") as f: + for line in f: + m = re.match(r"^([^\t#:=][^#:=]*?)\s*::?(?!=)", line) + if m and "=" not in m.group(1): + targets.update(m.group(1).split()) + return targets + + +def lanes(): + with open(".github/workflows/check.yml", encoding="utf-8") as f: + return set(re.findall(r"^\s*- lane: ([a-z0-9-]+)\s*$", f.read(), re.M)) + + +def repo_path(token): + token = re.sub(r"^\./", "", token).rstrip(",;.") + token = re.sub(r"#L\d+(-L?\d+)?$", "", token) + return re.sub(r":\d+(:\d+)?$", "", token) + + +os.chdir(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +TARGETS, LANES = make_targets(), lanes() +problems = [] + + +def report(md, kind, what): + problems.append(f"{md}: {kind} {what}") + + +def check(md): + here = os.path.dirname(md) + with open(md, encoding="utf-8") as f: + text = f.read() + prose_lines, code = [], [] + for fenced, line in blocks(text): + if line is None: + continue + if fenced: + code.append(("block", line)) + continue + prose, spans = code_spans(line) + prose_lines.append(prose) + code.extend(("span", s) for s in spans) + prose = "\n".join(prose_lines) + + links = re.findall( + r"\]\(\s*]+)>?(?:\s+(?:\"[^\"]*\"|'[^']*'))?\s*\)", prose + ) + links += re.findall(r"^ {0,3}\[[^\]]+\]:\s*?(?:\s|$)", prose, re.M) + for link in links: + if re.match(r"^[a-z][a-z0-9+.-]*:", link): + continue + file, _, anchor = link.partition("#") + target = os.path.normpath(os.path.join(here, file)) if file else md + if not os.path.exists(target): + report(md, "link", link) + elif anchor and target.endswith(".md") and anchor not in anchors(target): + report(md, "anchor", link) + + paths, roots, makes, flags = set(), set(), set(), set() + for kind, s in code: + for m in re.finditer( + r"(?:^|(?<=[\s\"'=(]))((?:\./)?" + ROOTS + r"/[^\s\"'`)]*)", s + ): + if not re.search(r"[*<>{}$\[\]]|\.\.\.", m.group(1)): + paths.add(repo_path(m.group(1))) + if kind == "span" and ROOT_FILE.fullmatch(s): + roots.add(s) + # Options and variable assignments before the target are skipped. + for m in re.finditer( + r"(?:^|(?<=[\s;&|(]))make(?: +(?:-\S+|[A-Za-z_][A-Za-z0-9_]*=\S*))*" + r" +([A-Za-z0-9_.][A-Za-z0-9_.-]*)(?![A-Za-z0-9_.=-])", + s, + ): + makes.add(m.group(1)) + for m in re.finditer( + r"(?:^|(?<=\s))(?:\./)?(scripts/[A-Za-z0-9_.-]+) (--[a-z0-9-]+)", s + ): + flags.add((m.group(1), m.group(2))) + gates = set(re.findall(r"gates \(([a-z0-9-]+)\)", text)) + + for path in sorted(paths): + if not os.path.exists(path): + report(md, "path", path) + for name in sorted(roots - UNTRACKED): + if not os.path.exists(name) and not os.path.exists(os.path.join(here, name)): + report(md, "file", name) + for target in sorted(makes - TARGETS): + report(md, "make", target) + for script, flag in sorted(flags): + try: + with open(script, encoding="utf-8") as f: + body = f.read() + except OSError: + continue + if not re.search( + r"(?` and `__init__` hooks +````md +``` +# inside four-backtick fence +``` +```` +~~~ +# inside tilde fence +~~~ +""" + +RESOLVING = """\ +# Fixture + +[a](target.md#foo-bar-baz) [b](target.md#the-make-indent-targetlane) +[c](target.md#repeat-1) [d](target.md#closing-hashes) +[e](target.md#indented-heading) [f](target.md#setext-heading) +[g](target.md#see-the-guide) [h](target.md#émigré-café) [i](target.md#a_b--cd) +[j](target.md "Title") [k](../../../src/main.rs#L10) [l](https://example.com/x) +[self](#fixture) `[in code](nowhere.md)` [r](target.md#the-id-and-__init__-hooks) +`src/main.rs:12` `src/main.rs#L5` `Cargo.toml` `TODO.md` `make fmt_all` +`make lint` `make -j4 V=1 lint` `scripts/test.sh --fixture` `gates (rust)` + +```sh +./scripts/test.sh +cat src/main.rs; ls src/main.rs, src/ +make hooks +[in a fence](nowhere.md) +``` +""" + +DANGLING = """\ +# Fixture + +[m](target.md#inside-four-backtick-fence) [n](target.md#inside-tilde-fence) +[o](target.md#repeat-2) [p](target.md#description-front-matter-is-not-a-heading) +[q](missing.md) + +[ref]: missing-ref.md + +`src/nope.rs:3` `Nope.toml` `make test2` `make indent-check` +`scripts/test.sh --fix` `gates (lane-2)` `make -k test3` + +```sh +scripts/missing.sh +make no_such_target +``` +""" + +DANGLING_REPORT = { + f".claude/skills/fixture/refs.md: {what}" + for what in ( + "anchor target.md#inside-four-backtick-fence", + "anchor target.md#inside-tilde-fence", + "anchor target.md#repeat-2", + "anchor target.md#description-front-matter-is-not-a-heading", + "link missing.md", + "link missing-ref.md", + "path src/nope.rs", + "path scripts/missing.sh", + "file Nope.toml", + "make test2", + "make test3", + "make indent-check", + "make no_such_target", + "flag scripts/test.sh --fix", + "CI lane lane-2", + ) +} + + +class SkillRefsTests(unittest.TestCase): + def check(self, refs): + with tempfile.TemporaryDirectory() as repo: + repo = Path(repo) + files = { + "Makefile": "hooks:\n\ttrue\nindent:\n\ttrue\nfmt_all:\n\ttrue\n" + "test lint: hooks\n\ttrue\nindent-check:= yes\n", + ".github/workflows/check.yml": "jobs:\n gates:\n" + " strategy:\n matrix:\n include:\n" + " - lane: rust\n - lane: portable\n", + "scripts/test.sh": "#!/bin/sh\ncase $1 in --fixture) ;; esac\n", + "src/main.rs": "fn main() {}\n", + "Cargo.toml": "[package]\n", + ".claude/skills/codetrial-verify/SKILL.md": "# Verify\n", + ".claude/skills/fixture/guide.md": "# Guide\n", + ".claude/skills/fixture/target.md": GOOD_TARGET, + ".claude/skills/fixture/refs.md": refs, + } + for name, text in files.items(): + (repo / name).parent.mkdir(parents=True, exist_ok=True) + (repo / name).write_text(text) + script = repo / "scripts/check-skill-refs.py" + shutil.copy(SKILL_REFS, script) + return subprocess.run( + [sys.executable, str(script)], cwd=repo, capture_output=True, text=True + ) + + def test_references_that_resolve_print_nothing(self): + result = self.check(RESOLVING) + self.assertEqual((result.returncode, result.stdout), (0, ""), result.stderr) + + def test_each_dangling_reference_is_named_once(self): + result = self.check(DANGLING) + self.assertEqual(result.returncode, 1, result.stderr) + lines = result.stdout.splitlines() + self.assertEqual(set(lines), DANGLING_REPORT) + self.assertEqual(len(lines), len(DANGLING_REPORT)) + + if __name__ == "__main__": unittest.main()