diff --git a/.claude/skills/codetrial-contribute/SKILL.md b/.claude/skills/codetrial-contribute/SKILL.md index 2c8f70b28..af1a7b037 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 000000000..122b87b91 --- /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 8452bcb9e..640042152 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/issue-sweep.sh b/.claude/skills/codetrial-triage/issue-sweep.sh index 91d0403d7..c4acb58dc 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 edfa157e9..90a0a3a78 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 941959c97..c6b972e74 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/.claude/skills/codetrial-triage/waiting.md b/.claude/skills/codetrial-triage/waiting.md index 57a0941be..288421e42 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 fbab05468..396c84471 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 (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. @@ -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 | @@ -214,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 0d0f2abf0..a199a4880 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. @@ -108,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 a8b22c575..000000000 --- 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/.claude/skills/codetrial-web/SKILL.md b/.claude/skills/codetrial-web/SKILL.md index 8bba10ff2..4bac9fc7c 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 000000000..2cfffaee6 --- /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). diff --git a/scripts/check-skill-refs.py b/scripts/check-skill-refs.py new file mode 100755 index 000000000..083339b38 --- /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"(? /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,12 @@ 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 skills name scripts, lanes, make targets and each other's headings, and an +# agent follows a dangling one without noticing. Their own scripts are tested +# like any other, against a fake gh and a scratch repository. +gate skill-refs "$PYTHON" "$ROOT/scripts/check-skill-refs.py" +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 000000000..cce2d772f --- /dev/null +++ b/tests/test_skill_scripts.py @@ -0,0 +1,411 @@ +#!/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. The reference checker runs +in a scratch repository whose Makefile, workflow and skills are written here, so +a case names exactly what resolves and what does not. +""" + +import importlib.util +import json +import os +import shutil +import subprocess +import sys +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" +SKILL_REFS = ROOT / "scripts" / "check-skill-refs.py" + +# 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"], []) + + +GOOD_TARGET = """\ +--- +name: fixture +description: front matter is not a heading +--- +# Foo, bar (baz) +## The `make indent` target/lane +## Repeat +## Repeat +## Closing hashes ## + ## Indented heading +Setext heading +-------------- +## See [the guide](guide.md) +## Émigré café +## a_b & c—d +## The `` 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()