diff --git a/README.md b/README.md index 220e74d7e..925a7f79e 100644 --- a/README.md +++ b/README.md @@ -334,13 +334,13 @@ Running the test suite (`cargo test`) also needs **`python3`** on `PATH`: the in ## Releasing -From a clone of this repo, signed in to `gh`, run the release script with the new version: +From a clone of this repo, signed in to `gh` and with `claude` logged in, run the release script with the new version: ```sh scripts/release.sh 0.4.0 ``` -It works from `origin/main` in a temporary worktree, so your checked-out branch and uncommitted changes don't matter and aren't touched. It opens a pull request from `release-` that bumps the version in `Cargo.toml` and `Cargo.lock` and nothing else, with GitHub's generated notes for the pull requests merged since the last tag and the version diff as its body. It waits for the pull request's checks, merges it with a merge commit, then tags that merge commit `v` and pushes the tag. It prints a line on stderr for each step. If the checks fail, it stops before merging and leaves the pull request open. +It works from `origin/main` in a temporary worktree, so your checked-out branch and uncommitted changes don't matter and aren't touched. It opens a pull request from `release-` that bumps the version in `Cargo.toml` and `Cargo.lock` and nothing else. Its body is a summary of the release, then the version diff. `claude -p`, with no tools, writes the summary from the version diff and the title, number and body of each pull request merged since the last tag, following the prompt in [`scripts/release-summary.md`](scripts/release-summary.md). If `claude` fails or prints nothing, the script warns and uses GitHub's generated notes as the summary instead, with a note saying so. It waits for the pull request's checks, merges it with a merge commit, then tags that merge commit `v` and pushes the tag. It prints a line on stderr for each step. If the checks fail, it stops before merging and leaves the pull request open. The tag starts the release workflow, generated by [`dist`](https://github.com/axodotdev/cargo-dist) from `dist-workspace.toml`. It builds the Linux and macOS binaries and the installer, checks that the Linux binary starts on Rocky Linux 9 and in WSL2 with Ubuntu 24.04, and only then publishes the GitHub Release. It then publishes the same version to crates.io, skipping it if it is already there, and finally replaces the Release's body with notes generated from the merged pull requests. diff --git a/scripts/release-summary.md b/scripts/release-summary.md new file mode 100644 index 000000000..a38382c38 --- /dev/null +++ b/scripts/release-summary.md @@ -0,0 +1,15 @@ +Write the summary of a thirdshift release for its release pull request and its GitHub Release page. Below this prompt are the release's version diff and the title, number and body of each pull request merged since the last release. + +Print only the summary, in GitHub Markdown, with no heading and nothing before or after it: + +1. A short headline paragraph on what the release is about: its biggest change for someone running `thirdshift`, in a sentence or two. +2. What changes for someone running `thirdshift`, first: new commands, then behaviour that is new or changed, most important first. +3. Site, docs and CI work in one short grouped line after that, such as "Also: site, docs and CI updates (#12, #14, #15)." Leave it out if there was none. + +Reference every change by its pull request number, as `#`. Several pull requests that make one change can share a bullet. + +Claim nothing the pull request titles and bodies don't say. Don't guess at motives, impact or details they leave out. When a title is all there is, say no more than the title does. + +Aim for the tone and density of the 0.3.0 summary: + +> 0.3.0 ships the changes merged since v0.2.0. The headline is the **Merge run** (`thirdshift merge `): #89 (define Merge run, Self-merge and Foreign commit), #96 (tracer bullet), #97 (a failed Self-merge goes back round the Repair loop; policy refusals leave the PR ready), #98 (post-merge steps: delete the Issue branch, close the issue), #99 (review Foreign commits with a Repair). Also #95 (Repair and base-move budgets raised to 5) and the site work in #72–#87. diff --git a/scripts/release.sh b/scripts/release.sh index 053b14a35..b2654ccea 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -6,9 +6,13 @@ # # Usage: scripts/release.sh # -# Run it in a clone of this repo, signed in to gh. It works from origin/main in -# a temporary worktree, so the branch checked out where it runs and any -# uncommitted changes there are neither used nor changed. +# Run it in a clone of this repo, signed in to gh and with claude logged in. It +# works from origin/main in a temporary worktree, so the branch checked out +# where it runs and any uncommitted changes there are neither used nor changed. +# +# claude, with no tools, writes the PR's summary from the prompt in +# release-summary.md beside this script. If it fails, the summary is GitHub's +# generated notes instead, and the script warns and carries on. # # Prints a progress line on stderr for each step. Exits 0 once the tag is # pushed; if the PR's checks fail, it leaves the PR open and exits 1. @@ -16,6 +20,9 @@ set -euo pipefail # So a failure inside $(…) stops the script too. shopt -s inherit_errexit +# The prompt the summary agent gets ahead of the release's input. +summary_prompt=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/release-summary.md + # Seconds between reads of the PR's workflow runs. poll_seconds=${RELEASE_POLL_SECONDS:-15} @@ -42,6 +49,7 @@ main() { head=$(git rev-parse HEAD) git push --quiet origin "HEAD:refs/heads/$branch" + progress "writing the summary with claude" # Built first so a failure to build it stops the script, as it would not # inside the gh command line. body=$(pr_body "$base" "$tag") @@ -102,21 +110,26 @@ rewrite() { # pr_body # The body of the bump PR on HEAD, which will name, off : the # summary, between markers the Release page step can find, then the version -# diff. For now the summary is GitHub's generated notes for the PRs merged -# since the last tag. +# diff. pr_body() { - local previous notes - previous=$(git describe --tags --abbrev=0 --match 'v[0-9]*' "$1" 2>/dev/null || true) - notes=$(gh api 'repos/{owner}/{repo}/releases/generate-notes' \ - -f tag_name="$2" -f target_commitish="$1" \ - ${previous:+-f previous_tag_name="$previous"} --jq .body) + local diff summary + diff=$(version_diff "$1") + summary=$(release_summary "$1" "$2" "$diff") cat <<-EOF ## Summary - $notes + $summary + $diff + EOF +} + +# version_diff +# The version diff section: HEAD's changes from . +version_diff() { + cat <<-EOF ## Version diff \`\`\`diff @@ -125,6 +138,48 @@ pr_body() { EOF } +# release_summary +# The agent's summary of the release, or, if the agent fails or prints +# nothing, a note saying so and GitHub's generated notes. +release_summary() { + local previous input summary + previous=$(git describe --tags --abbrev=0 --match 'v[0-9]*' "$1" 2>/dev/null || true) + input=$(summary_input "$1" "$previous" "$3") + # In an if, so the agent failing falls back rather than stopping the script. + if summary=$({ cat "$summary_prompt" && echo && echo "$input"; } | + claude -p --tools '' --strict-mcp-config) && + [ -n "${summary//[[:space:]]/}" ]; then + echo "$summary" + return + fi + progress "warning: the agent summary is unavailable, so the summary is GitHub's generated notes" + echo "_The agent summary was unavailable, so this is GitHub's generated notes._" + echo + gh api 'repos/{owner}/{repo}/releases/generate-notes' \ + -f tag_name="$2" -f target_commitish="$1" \ + ${previous:+-f previous_tag_name="$previous"} --jq .body +} + +# summary_input +# What the agent summarises: the version diff, then the title, number and body +# of each PR merged into since , oldest first. The bump PR +# isn't merged yet, so it isn't among them. +summary_input() { + local number title body + cat <<-EOF + $3 + + ## Pull requests merged since ${2:-the first commit} + EOF + git log --reverse --merges --format=%s "${2:+$2..}$1" | + sed -n 's/^Merge pull request #\([0-9][0-9]*\) from .*/\1/p' | + while read -r number; do + title=$(gh pr view "$number" --json title --jq .title) + body=$(gh pr view "$number" --json body --jq .body) + printf '\n### #%s: %s\n\n%s\n' "$number" "$title" "$body" + done +} + # wait_for_workflow_runs # Waits until has workflow runs and all of them have completed, then # prints the names of those that did not succeed, comma-separated, or nothing. diff --git a/tests/fakes/claude b/tests/fakes/claude index c6a1ec0b3..e78e1cb9f 100644 --- a/tests/fakes/claude +++ b/tests/fakes/claude @@ -21,6 +21,10 @@ The `init` line carries session id `fake-session-` for the n-th call. Lines the script appends to the file named by $FAKE_CLAUDE_AFTER_RESULT are emitted after the closing `result` line, e.g. to kill a background task as the session ends. + +Called without --output-format, as `claude -p` with its prompt on stdin, it +records stdin as `stdin` and prints only what the script prints, as print +mode's plain text output does. """ import json @@ -74,6 +78,7 @@ def main(): == 0 ) + text_mode = "--output-format" not in argv record_path = os.environ["FAKE_CLAUDE_RECORD"] records = [] if os.path.exists(record_path): @@ -88,11 +93,16 @@ def main(): "merging": merging, "plugin_dir": plugin_dir, "plugin_files": plugin_snapshot(plugin_dir) if plugin_dir else {}, + "stdin": sys.stdin.read() if text_mode else None, } ) with open(record_path, "w") as f: json.dump(records, f, indent=2) + script = script_for(records) + if text_mode: + sys.exit(subprocess.run(["bash", "-e", script]).returncode) + init = { "type": "system", "subtype": "init", @@ -100,7 +110,6 @@ def main(): "session_id": f"fake-session-{len(records)}", } print(json.dumps(init), flush=True) - script = script_for(records) # Beside the record, not in $TMPDIR, which tests expect to be left empty. after_result = f"{record_path}.after-result.{len(records)}" env = dict(os.environ, FAKE_CLAUDE_AFTER_RESULT=after_result) diff --git a/tests/fakes/gh b/tests/fakes/gh index a65bf7ba0..89eb102a9 100644 --- a/tests/fakes/gh +++ b/tests/fakes/gh @@ -159,15 +159,27 @@ def pr_create(state, flags): def pr_view(state, positional, flags): + """By head branch, its newest PR, or by number. `--jq .` prints + that one field raw, as gh does for a string.""" head = positional[0] - matches = [pr for pr in state["prs"] if pr["head"] == head] + if head.isdigit(): + matches = [pr for pr in state["prs"] if pr["number"] == int(head)] + else: + matches = [pr for pr in state["prs"] if pr["head"] == head] if not matches: die(f'no pull requests found for branch "{head}"') pr = matches[-1] wanted = flags["json"].split(",") + jq = flags.get("jq") + if jq is not None: + if not jq.startswith(".") or jq[1:] not in wanted: + die(f"fake gh: unsupported --jq {jq}", 2) + fields = dict(pr_fields(pr), title=pr["title"], body=pr.get("body", "")) + print(fields[jq[1:]]) + return fields = pr_fields(pr) if "headRefOid" in wanted: - fields["headRefOid"] = pr.get("headRefOid") or origin("rev-parse", f"refs/heads/{head}") + fields["headRefOid"] = pr.get("headRefOid") or origin("rev-parse", f"refs/heads/{pr['head']}") fields["closingIssuesReferences"] = [ {"number": int(number)} for number in CLOSING_KEYWORD.findall(pr.get("body", "")) ] diff --git a/tests/release.rs b/tests/release.rs index d3567d3c1..2fa996751 100644 --- a/tests/release.rs +++ b/tests/release.rs @@ -1,6 +1,7 @@ //! The maintainer's release script, `scripts/release.sh`, run as a black box -//! against a bare local origin with the fake `gh` on PATH and the user's git -//! configuration kept out, as the site deploy tests run the publish script. +//! against a bare local origin with the fake `gh` and `claude` on PATH and +//! the user's git configuration kept out, as the site deploy tests run the +//! publish script. //! //! The script, like the fake `gh` it drives here, relies on GNU tools, so its //! tests run on Linux only. @@ -58,8 +59,9 @@ fn with_version(file: &str, version: &str) -> String { } /// A bare origin whose main is at version 0.1.0, tagged `v0.1.0`, with one PR -/// merged before the tag and one after; a contributor's clone that makes -/// them; the maintainer's clone the script runs in; and the fake `gh`'s state. +/// merged before the tag and two after; a contributor's clone that makes +/// them; the maintainer's clone the script runs in; the fake `gh`'s state; +/// and a fake `claude` that prints `AGENT_SUMMARY`. struct Release { temp: TempDir, } @@ -100,11 +102,17 @@ impl Release { git(&contributor, &["commit", "-q", "-m", "Initial"]); git(&contributor, &["push", "-q", "origin", "HEAD:main"]); - release.merge_pr("before-tag", "Shipped in 0.1.0"); + release.merge_pr("before-tag", "Shipped in 0.1.0", "Old news."); git(&contributor, &["pull", "-q", "origin", "main"]); git(&contributor, &["tag", "v0.1.0"]); git(&contributor, &["push", "-q", "origin", "v0.1.0"]); - release.merge_pr("after-tag", "Add the frobnicator"); + release.merge_pr( + "after-tag", + "Add the frobnicator", + "It frobs.\n\nCloses #7.", + ); + release.merge_pr("site", "Tidy the site", ""); + release.agent_runs(&format!("printf '%s\\n' '{AGENT_SUMMARY}'")); git(root, &["clone", "-q", "origin.git", "maintainer"]); release @@ -145,9 +153,9 @@ impl Release { String::from_utf8(output.stdout).unwrap().trim().to_owned() } - /// Opens a PR titled `title` from a new branch `head` off main and merges - /// it with a merge commit, through the fake `gh`. - fn merge_pr(&self, head: &str, title: &str) { + /// Opens a PR titled `title` with `body` from a new branch `head` off main + /// and merges it with a merge commit, through the fake `gh`. + fn merge_pr(&self, head: &str, title: &str, body: &str) { let contributor = self.contributor(); git(&contributor, &["checkout", "-q", "-b", head, "origin/main"]); fs::write(contributor.join(format!("{head}.txt")), head).unwrap(); @@ -156,7 +164,7 @@ impl Release { git(&contributor, &["push", "-q", "origin", head]); let sha = git(&contributor, &["rev-parse", "HEAD"]); self.gh(&[ - "pr", "create", "--head", head, "--base", "main", "--title", title, "--body", "", + "pr", "create", "--head", head, "--base", "main", "--title", title, "--body", body, ]); self.gh(&["pr", "merge", head, "--merge", "--match-head-commit", &sha]); git(&contributor, &["fetch", "-q", "origin"]); @@ -173,6 +181,24 @@ impl Release { ]); } + /// Makes the fake `claude` run the bash `script`. + fn agent_runs(&self, script: &str) { + fs::write(self.root().join("claude-script.sh"), script).unwrap(); + } + + /// The fake `claude`'s calls, oldest first. + fn agent_calls(&self) -> Vec { + let path = self.root().join("claude-calls.json"); + if !path.exists() { + return Vec::new(); + } + serde_json::from_str::(&fs::read_to_string(path).unwrap()) + .unwrap() + .as_array() + .unwrap() + .clone() + } + fn command(&self, program: &Path, dir: &Path) -> Command { let path = format!( "{}:{}", @@ -185,6 +211,8 @@ impl Release { .env("PATH", path) .env("FAKE_GH_STATE", self.root().join("gh-state.json")) .env("FAKE_GH_RECORD", self.root().join("gh-calls.json")) + .env("FAKE_CLAUDE_SCRIPT", self.root().join("claude-script.sh")) + .env("FAKE_CLAUDE_RECORD", self.root().join("claude-calls.json")) .env("RELEASE_POLL_SECONDS", "0"); command } @@ -219,18 +247,32 @@ impl Release { self.pr(head)["headRefOid"].as_str().unwrap().to_owned() } + fn pr_body(&self, head: &str) -> String { + self.pr(head)["body"].as_str().unwrap().to_owned() + } + fn origin_tag(&self, tag: &str) -> Option { let refs = self.origin(&["tag", "--list", tag]); (!refs.is_empty()).then(|| self.origin(&["rev-parse", &format!("{tag}^{{commit}}")])) } } +/// The summary section of a bump PR's `body`, between its markers, and what +/// follows it. +fn summary_and_rest(body: &str) -> (&str, &str) { + let start_marker = "\n"; + let start = body.find(start_marker).expect(body) + start_marker.len(); + let end = body.find("").expect(body); + (&body[start..end], &body[end..]) +} + fn stderr(output: &Output) -> String { String::from_utf8_lossy(&output.stderr).into_owned() } const GREEN: &str = r#"[{"name": "test", "conclusion": "success", "pending_polls": 2}]"#; const RED: &str = r#"[{"name": "test", "conclusion": "failure"}]"#; +const AGENT_SUMMARY: &str = "The headline is the frobnicator (#2)."; #[test] #[cfg(target_os = "linux")] @@ -272,30 +314,120 @@ fn a_release_bumps_only_the_version_merges_the_pr_and_tags_the_merge_commit() { #[test] #[cfg(target_os = "linux")] -fn the_pr_body_has_a_marked_summary_of_the_generated_notes_then_the_version_diff() { +fn the_agent_gets_the_prompt_file_then_the_version_diff_and_the_prs_merged_since_the_last_tag() { let release = Release::new(); release.ci_reports(GREEN); + let main_before = release.origin(&["rev-parse", "main"]); let output = release.run_script("0.2.0"); assert!(output.status.success(), "{}", stderr(&output)); - let body = release.pr("release-0.2.0")["body"] - .as_str() + let calls = release.agent_calls(); + assert_eq!(calls.len(), 1, "{calls:#?}"); + let head = release.pr_head("release-0.2.0"); + let diff = release.origin(&["diff", "--no-color", "--unified=1", &main_before, &head]); + let prompt = fs::read_to_string(manifest_dir().join("scripts/release-summary.md")).unwrap(); + let expected = format!( + "{prompt} +## Version diff + +```diff +{diff} +``` + +## Pull requests merged since v0.1.0 + +### #2: Add the frobnicator + +It frobs. + +Closes #7. + +### #3: Tidy the site +" + ); + assert_eq!(calls[0]["stdin"].as_str().unwrap(), expected); +} + +#[test] +#[cfg(target_os = "linux")] +fn the_agent_runs_in_print_mode_with_tools_disabled() { + let release = Release::new(); + release.ci_reports(GREEN); + + let output = release.run_script("0.2.0"); + + assert!(output.status.success(), "{}", stderr(&output)); + let argv: Vec = release.agent_calls()[0]["argv"] + .as_array() .unwrap() - .to_owned(); - let start = body.find("").expect(&body); - let end = body.find("").expect(&body); - let summary = &body[start..end]; - assert!(summary.contains("Add the frobnicator"), "{body}"); - assert!(!summary.contains("Shipped in 0.1.0"), "{body}"); - let diff = &body[end..]; - assert!(diff.contains("```diff"), "{body}"); + .iter() + .map(|arg| arg.as_str().unwrap().to_owned()) + .collect(); + assert!(argv.contains(&"-p".to_owned()), "{argv:?}"); + let tools = argv + .iter() + .position(|arg| arg == "--tools") + .expect("no --tools"); + assert_eq!(argv[tools + 1], "", "{argv:?}"); + assert!(argv.contains(&"--strict-mcp-config".to_owned()), "{argv:?}"); +} + +#[test] +#[cfg(target_os = "linux")] +fn the_pr_body_has_the_agents_summary_in_a_marked_section_then_the_version_diff() { + let release = Release::new(); + release.ci_reports(GREEN); + + let output = release.run_script("0.2.0"); + + assert!(output.status.success(), "{}", stderr(&output)); + let body = release.pr_body("release-0.2.0"); + let (summary, rest) = summary_and_rest(&body); + assert_eq!(summary, format!("{AGENT_SUMMARY}\n"), "{body}"); + assert!(rest.contains("## Version diff\n\n```diff"), "{body}"); assert!( - diff.contains("-version = \"0.1.0\"\n+version = \"0.2.0\""), + rest.contains("-version = \"0.1.0\"\n+version = \"0.2.0\""), "{body}" ); } +#[test] +#[cfg(target_os = "linux")] +fn when_the_agent_fails_or_prints_nothing_the_summary_is_the_generated_notes() { + for (case, script) in [ + ("fails", "echo 'Not logged in' >&2; exit 1"), + ("prints nothing", "printf '\\n'"), + ] { + let release = Release::new(); + release.ci_reports(GREEN); + release.agent_runs(script); + + let output = release.run_script("0.2.0"); + + assert!(output.status.success(), "{case}: {}", stderr(&output)); + let err = stderr(&output); + assert!( + err.lines() + .any(|line| line.starts_with("release: warning:") && line.contains("agent summary")), + "{case}: {err}" + ); + let body = release.pr_body("release-0.2.0"); + let (summary, rest) = summary_and_rest(&body); + let (note, notes) = summary.split_once("\n\n").expect(&body); + assert!( + !note.contains('\n') && note.contains("agent summary was unavailable"), + "{case}: {body}" + ); + assert!(notes.starts_with("## What's Changed\n"), "{case}: {body}"); + assert!(notes.contains("Add the frobnicator"), "{case}: {body}"); + assert!(!notes.contains("Shipped in 0.1.0"), "{case}: {body}"); + assert!(rest.contains("```diff"), "{case}: {body}"); + let merge = release.origin(&["rev-parse", "main"]); + assert_eq!(release.origin_tag("v0.2.0"), Some(merge), "{case}"); + } +} + #[test] #[cfg(target_os = "linux")] fn each_step_prints_a_progress_line_on_stderr() { @@ -306,8 +438,9 @@ fn each_step_prints_a_progress_line_on_stderr() { assert!(output.status.success(), "{}", stderr(&output)); let merge = release.origin(&["rev-parse", "main"]); - let url = format!("https://github.com/{REPO}/pull/3"); + let url = format!("https://github.com/{REPO}/pull/4"); let expected = [ + "release: writing the summary with claude".to_owned(), format!("release: opened {url}"), format!("release: waiting for CI on {url}"), format!("release: merged {url} as {}", &merge[..7]), @@ -393,6 +526,7 @@ fn the_maintainers_checkout_neither_affects_the_release_nor_is_changed() { fn a_failure_to_generate_the_notes_stops_before_the_pr_is_opened() { let release = Release::new(); release.ci_reports(GREEN); + release.agent_runs("exit 1"); release.gh(&[ "fake", "fails",