From ffdd5fec4921c31f16fb1c73b367de8d999eaea1 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Fri, 28 Aug 2026 19:14:47 -0400 Subject: [PATCH 1/9] feat(gh): read GitHub issues through the gh client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the issue half of the `gh` boundary, so a later `wt issue` can fetch an issue's title, body, labels, type and milestone without reaching past the `GhClient` seam. `list_open_issues` and `view_issue` are *defaulted* trait methods that return `Error::GhUnavailable`, rather than required ones: a third-party `GhClient` implementation stays source-compatible and only opts in when it actually supports the issue workflow. Every optional field on the new types carries `#[serde(default)]`, so a sparse `gh` response — a repository that uses no issue types, labels or milestones — parses rather than failing. The requested field set is deliberately narrow: comments, assignees, reactions and project data are not fetched, since nothing downstream reads them and they dominate the token cost of an issue. Claude-Session: https://claude.ai/code/session_014ce9P64RKbVU6kj3GmA1mo --- src/gh/mod.rs | 55 +++++++++++++++++++- src/gh/types.rs | 136 +++++++++++++++++++++++++++++++++++++++++++++++- src/testutil.rs | 37 ++++++++++++- 3 files changed, 225 insertions(+), 3 deletions(-) diff --git a/src/gh/mod.rs b/src/gh/mod.rs index f795b03..ffebf7e 100644 --- a/src/gh/mod.rs +++ b/src/gh/mod.rs @@ -9,10 +9,32 @@ use std::path::Path; use std::process::Command; use crate::error::{Error, Result}; -pub use types::{Author, OpenPr, PrSummary, PrView, pr_state}; +pub use types::{ + Author, IssueLabel, IssueMilestone, IssueSummary, IssueType, IssueView, OpenPr, PrSummary, + PrView, pr_state, +}; /// Performs GitHub pull-request operations via `gh`. pub trait GhClient { + /// Lists open issues for the repository at `dir`. + /// + /// Defaulted so third-party [`GhClient`] implementations stay + /// source-compatible; clients that support the issue workflow override it. + fn list_open_issues(&self, _dir: &Path) -> Result> { + Err(Error::GhUnavailable( + "GitHub issue operations are not supported by this client".into(), + )) + } + + /// Views the issue identified by `target` (a number or URL). + /// + /// Defaulted for the same reason as [`GhClient::list_open_issues`]. + fn view_issue(&self, _dir: &Path, _target: &str) -> Result { + Err(Error::GhUnavailable( + "GitHub issue operations are not supported by this client".into(), + )) + } + /// Lists open PRs for the repository at `dir`. fn list_open_prs(&self, dir: &Path) -> Result>; @@ -51,6 +73,37 @@ pub trait GhClient { pub struct RealGh; impl GhClient for RealGh { + fn list_open_issues(&self, dir: &Path) -> Result> { + let output = run_gh( + dir, + &[ + "issue", + "list", + "--state", + "open", + "--limit", + "100", + "--json", + "number,title,state,labels,issueType,milestone,createdAt,url", + ], + )?; + serde_json::from_str(&output).map_err(Error::from) + } + + fn view_issue(&self, dir: &Path, target: &str) -> Result { + let output = run_gh( + dir, + &[ + "issue", + "view", + target, + "--json", + "number,title,body,state,labels,issueType,milestone,createdAt,updatedAt,url", + ], + )?; + serde_json::from_str(&output).map_err(Error::from) + } + fn list_open_prs(&self, dir: &Path) -> Result> { let output = run_gh( dir, diff --git a/src/gh/types.rs b/src/gh/types.rs index 9d97114..84e9816 100644 --- a/src/gh/types.rs +++ b/src/gh/types.rs @@ -1,9 +1,96 @@ //! `gh` JSON shapes and their mapping to the domain model (spec §4). -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use crate::model::PrState; +/// A GitHub issue label. +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize, Serialize)] +pub struct IssueLabel { + /// Label name. + #[serde(default)] + pub name: String, +} + +/// A GitHub issue type (the organization-defined "type" field, distinct from +/// labels). +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize, Serialize)] +pub struct IssueType { + /// Issue type name. + #[serde(default)] + pub name: String, +} + +/// A GitHub issue milestone. +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize, Serialize)] +pub struct IssueMilestone { + /// Milestone title. + #[serde(default)] + pub title: String, +} + +/// An open issue as returned by `gh issue list --json ...`. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub struct IssueSummary { + /// Issue number. + pub number: u64, + /// Issue title. + pub title: String, + /// Issue state (`OPEN`/`CLOSED`). + pub state: String, + /// Labels attached to the issue. + #[serde(default)] + pub labels: Vec, + /// Optional organization-defined issue type. + #[serde(rename = "issueType", default)] + pub issue_type: Option, + /// Optional milestone. + #[serde(default)] + pub milestone: Option, + /// ISO-8601 creation time. + #[serde(rename = "createdAt", default)] + pub created_at: String, + /// Issue web URL. + #[serde(default)] + pub url: String, +} + +/// A full issue as returned by `gh issue view --json ...`. +/// +/// This carries only the token-efficient issue context the generation step +/// needs; comments, assignees, reactions and project bookkeeping are +/// deliberately not requested. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub struct IssueView { + /// Issue number. + pub number: u64, + /// Issue title. + pub title: String, + /// Issue body. + #[serde(default)] + pub body: String, + /// Issue state (`OPEN`/`CLOSED`). + pub state: String, + /// Labels attached to the issue. + #[serde(default)] + pub labels: Vec, + /// Optional organization-defined issue type. + #[serde(rename = "issueType", default)] + pub issue_type: Option, + /// Optional milestone. + #[serde(default)] + pub milestone: Option, + /// ISO-8601 creation time. + #[serde(rename = "createdAt", default)] + pub created_at: String, + /// ISO-8601 update time. + #[serde(rename = "updatedAt", default)] + pub updated_at: String, + /// Issue web URL. + #[serde(default)] + pub url: String, +} + /// A PR author (`{ "login": ... }`). #[derive(Debug, Clone, Default, Deserialize)] pub struct Author { @@ -124,6 +211,53 @@ pub fn pr_state(state: &str, is_draft: bool) -> PrState { mod tests { use super::*; + #[test] + fn parses_issue_list_json() { + let json = r#"[ + {"number": 12, "title": "Broken login", "state": "OPEN", + "labels": [{"name": "bug"}], "issueType": {"name": "Bug"}, + "milestone": {"title": "v2"}, "createdAt": "2024-01-15T10:30:00Z", + "url": "https://github.com/o/r/issues/12"}, + {"number": 13, "title": "Bare", "state": "OPEN"} + ]"#; + let issues: Vec = serde_json::from_str(json).unwrap(); + assert_eq!(issues.len(), 2); + assert_eq!(issues[0].number, 12); + assert_eq!(issues[0].labels[0].name, "bug"); + assert_eq!(issues[0].issue_type.as_ref().unwrap().name, "Bug"); + assert_eq!(issues[0].milestone.as_ref().unwrap().title, "v2"); + // Every optional field defaults, so a sparse `gh` response still parses. + assert!(issues[1].labels.is_empty()); + assert_eq!(issues[1].issue_type, None); + assert_eq!(issues[1].milestone, None); + assert_eq!(issues[1].url, ""); + } + + #[test] + fn parses_issue_view_json() { + let json = r#"{"number": 7, "title": "Add login", "body": "Please add it.", + "state": "OPEN", "labels": [{"name": "enhancement"}], + "issueType": null, "milestone": null, + "createdAt": "2024-01-15T10:30:00Z", "updatedAt": "2024-02-01T09:00:00Z", + "url": "https://github.com/o/r/issues/7"}"#; + let issue: IssueView = serde_json::from_str(json).unwrap(); + assert_eq!(issue.number, 7); + assert_eq!(issue.body, "Please add it."); + assert_eq!(issue.labels[0].name, "enhancement"); + assert_eq!(issue.issue_type, None); + assert_eq!(issue.url, "https://github.com/o/r/issues/7"); + } + + #[test] + fn issue_view_tolerates_a_body_only_response() { + // `gh` omits fields the repository does not use; only number/title/state + // are required, so the rest must not make parsing fail. + let issue: IssueView = + serde_json::from_str(r#"{"number": 1, "title": "T", "state": "OPEN"}"#).unwrap(); + assert_eq!(issue.body, ""); + assert_eq!(issue.updated_at, ""); + } + #[test] fn parses_pr_list_json() { let json = r#"[ diff --git a/src/testutil.rs b/src/testutil.rs index b4663bf..23efeb7 100644 --- a/src/testutil.rs +++ b/src/testutil.rs @@ -17,7 +17,7 @@ use std::collections::VecDeque; use crate::agent::{AgentClient, AgentKind, AgentOptions, AgentRun, AgentVersion, DetectedAgent}; use crate::cx::{Cx, Env, Input, Stream}; use crate::error::Error; -use crate::gh::{GhClient, OpenPr, PrSummary, PrView, RealGh}; +use crate::gh::{GhClient, IssueSummary, IssueView, OpenPr, PrSummary, PrView, RealGh}; use crate::git::cli::{GitCli, RealGit}; /// A fake [`GhClient`] returning canned PR data or simulating an unavailable @@ -33,6 +33,8 @@ pub(crate) struct FakeGh { edit_stdout: String, create_args: Arc>>>, edit_args: Arc>>>, + issue_list: Vec, + issue: Option, } #[allow(dead_code)] @@ -65,6 +67,22 @@ impl FakeGh { } } + /// A fake that returns `issue` from `view_issue`. + pub(crate) fn with_issue(issue: IssueView) -> Self { + FakeGh { + issue: Some(issue), + available: true, + ..Default::default() + } + } + + /// Sets the issues returned by `list_open_issues`. + pub(crate) fn with_issue_list(mut self, list: Vec) -> Self { + self.issue_list = list; + self.available = true; + self + } + /// A fake simulating a missing/unauthenticated `gh`. pub(crate) fn unavailable() -> Self { FakeGh::default() @@ -94,6 +112,23 @@ impl FakeGh { } impl GhClient for FakeGh { + fn list_open_issues(&self, _dir: &std::path::Path) -> crate::error::Result> { + if self.available { + Ok(self.issue_list.clone()) + } else { + Err(Error::GhUnavailable("gh unavailable".into())) + } + } + + fn view_issue(&self, _dir: &std::path::Path, _target: &str) -> crate::error::Result { + if !self.available { + return Err(Error::GhUnavailable("gh unavailable".into())); + } + self.issue + .clone() + .ok_or_else(|| Error::operation("no issue configured")) + } + fn list_open_prs(&self, _dir: &std::path::Path) -> crate::error::Result> { if self.available { Ok(self.list.clone()) From 678e23223eccec12f8e6e5e0e682b26aadf39558 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Fri, 28 Aug 2026 19:16:41 -0400 Subject: [PATCH 2/9] feat(model): record a linked GitHub issue on worktree rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the `wt..issue*` metadata a worktree needs to remember which GitHub issue it belongs to, and surfaces it on the row model. The link deliberately lives in git config rather than in the branch name, so a branch that does not follow the `TYPE/{number}-SLUG` convention is still resolvable back to its issue. `issueBrief` persists the generated implementation brief so an embedder (karet) can read it back instead of paying to regenerate it. `Column::Issue` is deliberately absent from `Column::ALL`: it is opt-in via `list.columns`, so the default table is unchanged. A test pins that intent, since adding the variant to `ALL` is an easy and silent mistake. `MetaUpdate` gains the four matching fields, and the two existing PR call sites now spread `..MetaUpdate::default()` rather than naming every field — a PR checkout leaves an existing issue link untouched instead of clobbering it. No `wt.schema` bump: the keys are purely additive, and `read_meta` maps missing keys to `None`, which `SCHEMA_VERSION`'s own contract names as the additive-safe case. `clear_meta` needs no change either, since it removes the whole `wt.` section; its test now writes every key the section can hold to prove that. Claude-Session: https://claude.ai/code/session_014ce9P64RKbVU6kj3GmA1mo --- src/commands/pr.rs | 3 ++ src/commands/pr_open.rs | 3 ++ src/config/wtconfig.rs | 103 ++++++++++++++++++++++++++++++++++++++++ src/model.rs | 36 +++++++++++++- src/output/render.rs | 26 +++++++++- src/output/table.rs | 1 + src/worktree/rows.rs | 16 ++++++- src/worktree/service.rs | 20 ++++++++ 8 files changed, 205 insertions(+), 3 deletions(-) diff --git a/src/commands/pr.rs b/src/commands/pr.rs index 410d9f5..2f94453 100644 --- a/src/commands/pr.rs +++ b/src/commands/pr.rs @@ -326,6 +326,9 @@ fn pr_meta(view: &crate::gh::PrView, base: &str, created_here: bool) -> MetaUpda pr_title: Some(view.title.clone()), pr_url: Some(view.url.clone()), created_by_wt: created_here, + // A PR checkout records no issue link; `None` leaves any existing one + // (e.g. from `wt issue`) untouched. + ..MetaUpdate::default() } } diff --git a/src/commands/pr_open.rs b/src/commands/pr_open.rs index 85de561..ae2f774 100644 --- a/src/commands/pr_open.rs +++ b/src/commands/pr_open.rs @@ -440,6 +440,9 @@ pub(crate) fn record_pr_metadata( // rather than recording a blank. pr_url: (!outcome.url.is_empty()).then(|| outcome.url.clone()), created_by_wt: false, + // Opening a PR records no issue link; `None` leaves any existing one + // (e.g. from `wt issue`) untouched. + ..MetaUpdate::default() }; let _lock = lock_repo(root)?; apply_meta(git, root, branch, &update) diff --git a/src/config/wtconfig.rs b/src/config/wtconfig.rs index a89938f..bb5561b 100644 --- a/src/config/wtconfig.rs +++ b/src/config/wtconfig.rs @@ -74,6 +74,15 @@ pub struct WtMeta { pub pr_url: Option, /// Whether the branch/worktree was created by `wt` (§10). pub created_by_wt: bool, + /// Linked GitHub issue number, for issue worktrees (issue #100). + pub issue_number: Option, + /// Cached issue title, so `wt list` can show it offline. + pub issue_title: Option, + /// Cached issue URL. + pub issue_url: Option, + /// The generated implementation brief. Persisted so an embedder (karet) + /// can read it instead of regenerating it. + pub issue_brief: Option, } /// The config key for `wt..`. @@ -102,6 +111,18 @@ pub fn read_meta(repo: &gix::Repository, branch: &str) -> WtMeta { let created_by_wt = config .boolean(key(branch, "createdByWt").as_str()) .unwrap_or(false); + let issue_number = config + .string(key(branch, "issueNumber").as_str()) + .and_then(|v| v.to_string().parse::().ok()); + let issue_title = config + .string(key(branch, "issueTitle").as_str()) + .map(|v| v.to_string()); + let issue_url = config + .string(key(branch, "issueUrl").as_str()) + .map(|v| v.to_string()); + let issue_brief = config + .string(key(branch, "issueBrief").as_str()) + .map(|v| v.to_string()); WtMeta { base_ref, pr_number, @@ -109,6 +130,10 @@ pub fn read_meta(repo: &gix::Repository, branch: &str) -> WtMeta { pr_title, pr_url, created_by_wt, + issue_number, + issue_title, + issue_url, + issue_brief, } } @@ -170,6 +195,48 @@ pub fn write_pr_number( Ok(()) } +/// Records the linked issue number for `branch`. +pub fn write_issue_number( + git: &dyn GitCli, + repo_root: &Path, + branch: &str, + number: u64, +) -> Result<()> { + git.run( + repo_root, + &["config", &key(branch, "issueNumber"), &number.to_string()], + )?; + Ok(()) +} + +/// Records the cached issue title for `branch`. +pub fn write_issue_title( + git: &dyn GitCli, + repo_root: &Path, + branch: &str, + title: &str, +) -> Result<()> { + git.run(repo_root, &["config", &key(branch, "issueTitle"), title])?; + Ok(()) +} + +/// Records the issue URL for `branch`. +pub fn write_issue_url(git: &dyn GitCli, repo_root: &Path, branch: &str, url: &str) -> Result<()> { + git.run(repo_root, &["config", &key(branch, "issueUrl"), url])?; + Ok(()) +} + +/// Records the generated implementation brief for `branch`. +pub fn write_issue_brief( + git: &dyn GitCli, + repo_root: &Path, + branch: &str, + brief: &str, +) -> Result<()> { + git.run(repo_root, &["config", &key(branch, "issueBrief"), brief])?; + Ok(()) +} + /// Marks `branch` as created by `wt`. pub fn mark_created_by_wt(git: &dyn GitCli, repo_root: &Path, branch: &str) -> Result<()> { git.run(repo_root, &["config", &key(branch, "createdByWt"), "true"])?; @@ -298,11 +365,47 @@ mod tests { } } + #[test] + fn issue_link_round_trips() { + let repo = TestRepo::init(); + write_issue_number(&RealGit, repo.root(), "topic", 7).unwrap(); + write_issue_title(&RealGit, repo.root(), "topic", "Add login").unwrap(); + write_issue_url(&RealGit, repo.root(), "topic", "https://example.com/7").unwrap(); + write_issue_brief(&RealGit, repo.root(), "topic", "Wire up the form.").unwrap(); + let got = meta(&repo, "topic"); + assert_eq!(got.issue_number, Some(7)); + assert_eq!(got.issue_title.as_deref(), Some("Add login")); + assert_eq!(got.issue_url.as_deref(), Some("https://example.com/7")); + assert_eq!(got.issue_brief.as_deref(), Some("Wire up the form.")); + } + + #[test] + fn issue_keys_are_absent_until_written() { + // The issue keys are purely additive: a repository written by an older + // build has none of them, and every one must read back as `None` rather + // than fail. This is what makes them safe without a `wt.schema` bump. + let repo = TestRepo::init(); + write_base_ref(&RealGit, repo.root(), "topic", "main").unwrap(); + let got = meta(&repo, "topic"); + assert_eq!(got.issue_number, None); + assert_eq!(got.issue_title, None); + assert_eq!(got.issue_url, None); + assert_eq!(got.issue_brief, None); + } + #[test] fn clear_removes_all_metadata() { let repo = TestRepo::init(); write_base_ref(&RealGit, repo.root(), "topic", "main").unwrap(); mark_created_by_wt(&RealGit, repo.root(), "topic").unwrap(); + // Every key the section can hold, so `--remove-section` is proven to + // clear the issue keys too and not just the ones it predates. + write_pr(&RealGit, repo.root(), "topic", 42, "open", "Title").unwrap(); + write_pr_url(&RealGit, repo.root(), "topic", "https://example.com/42").unwrap(); + write_issue_number(&RealGit, repo.root(), "topic", 7).unwrap(); + write_issue_title(&RealGit, repo.root(), "topic", "Add login").unwrap(); + write_issue_url(&RealGit, repo.root(), "topic", "https://example.com/7").unwrap(); + write_issue_brief(&RealGit, repo.root(), "topic", "Wire up the form.").unwrap(); clear_meta(&RealGit, repo.root(), "topic").unwrap(); assert_eq!(meta(&repo, "topic"), WtMeta::default()); // Clearing again (no section) is not an error. diff --git a/src/model.rs b/src/model.rs index 2b29730..3f76354 100644 --- a/src/model.rs +++ b/src/model.rs @@ -53,6 +53,8 @@ pub struct Worktree { pub commit: Option, /// Recorded pull request; `None` when none. pub pr: Option, + /// Linked GitHub issue; `None` when none (issue #100). + pub issue: Option, /// Whether a checked-out worktree exists for this row. `false` marks a /// "branch row": a local branch with no worktree, listed beneath the real /// worktrees with its ahead/behind relative to its base (issue #47). Not part @@ -103,6 +105,7 @@ impl Worktree { base_ref: None, commit: None, pr: None, + issue: None, has_worktree: true, recent_commits: Vec::new(), pr_url: None, @@ -166,6 +169,21 @@ pub struct Pr { pub title: String, } +/// A linked GitHub issue, recorded by `wt issue` (issue #100). +/// +/// The link lives in `wt..issue*` git config rather than in the branch +/// name, so a branch that does not follow the `TYPE/{number}-SLUG` convention is +/// still resolvable back to its issue. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct IssueLink { + /// Issue number. + pub number: u64, + /// Issue title. + pub title: String, + /// Issue web URL. + pub url: String, +} + /// Pull-request state, mirroring `gh` (spec §7). #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] #[serde(rename_all = "lowercase")] @@ -293,6 +311,9 @@ pub enum Column { Commit, /// PR number and state. Pr, + /// Linked issue number. Deliberately absent from [`Column::ALL`]: it is + /// opt-in via `list.columns`, so the default table is unchanged. + Issue, } impl Column { @@ -317,6 +338,7 @@ impl Column { "ahead-behind" => Column::AheadBehind, "commit" => Column::Commit, "pr" => Column::Pr, + "issue" => Column::Issue, _ => return None, }) } @@ -331,6 +353,7 @@ impl Column { Column::AheadBehind => "ahead-behind", Column::Commit => "commit", Column::Pr => "pr", + Column::Issue => "issue", } } } @@ -361,7 +384,8 @@ mod tests { "author": "Alice", "timestamp": "2024-01-15T10:30:00Z" }, - "pr": { "number": 42, "state": "open", "title": "Add login page" } + "pr": { "number": 42, "state": "open", "title": "Add login page" }, + "issue": null }"#; fn spec_example_worktree() -> Worktree { @@ -391,6 +415,7 @@ mod tests { state: PrState::Open, title: "Add login page".into(), }), + issue: None, has_worktree: true, recent_commits: Vec::new(), pr_url: None, @@ -398,6 +423,15 @@ mod tests { } } + #[test] + fn issue_column_parses_but_is_not_a_default_column() { + assert_eq!(Column::parse("issue"), Some(Column::Issue)); + assert_eq!(Column::Issue.identifier(), "issue"); + // Deliberate: `issue` is opt-in via `list.columns`, so adding it to + // `ALL` would silently widen every default table (issue #100). + assert!(!Column::ALL.contains(&Column::Issue)); + } + #[test] fn serializes_to_spec_schema() { let got: serde_json::Value = serde_json::to_value(spec_example_worktree()).unwrap(); diff --git a/src/output/render.rs b/src/output/render.rs index 2aee711..cd4eb1c 100644 --- a/src/output/render.rs +++ b/src/output/render.rs @@ -71,6 +71,14 @@ pub fn pr_cell(worktree: &Worktree) -> String { } } +/// The issue cell: `#`, or empty when the branch has no linked issue. +pub fn issue_cell(worktree: &Worktree) -> String { + match &worktree.issue { + Some(issue) => format!("#{}", issue.number), + None => String::new(), + } +} + /// The path cell: relative to the repo root, or absolute if outside it. pub fn path_cell(worktree: &Worktree, repo_root: &Path) -> String { match worktree.path.strip_prefix(repo_root) { @@ -103,6 +111,7 @@ pub fn cell(worktree: &Worktree, column: Column, ctx: &RenderCtx) -> String { Column::AheadBehind => ahead_behind_cell(worktree), Column::Commit => commit_cell(worktree, ctx.now), Column::Pr => pr_cell(worktree), + Column::Issue => issue_cell(worktree), } } @@ -142,6 +151,9 @@ pub(crate) fn status_block(worktree: &Worktree, entries: &[StatusEntry]) -> Stri pr.title ); } + if let Some(issue) = &worktree.issue { + let _ = writeln!(out, "issue: #{} \"{}\"", issue.number, issue.title); + } if !entries.is_empty() { let _ = writeln!(out, "dirty:"); for entry in entries { @@ -154,7 +166,7 @@ pub(crate) fn status_block(worktree: &Worktree, entries: &[StatusEntry]) -> Stri #[cfg(test)] mod tests { use super::*; - use crate::model::{Commit, Pr, PrState}; + use crate::model::{Commit, IssueLink, Pr, PrState}; use std::path::PathBuf; fn base() -> Worktree { @@ -222,6 +234,18 @@ mod tests { assert_eq!(path_cell(&w, root), "/elsewhere/y"); } + #[test] + fn issue_cell_renders_the_linked_number() { + let mut w = base(); + assert_eq!(issue_cell(&w), ""); + w.issue = Some(IssueLink { + number: 7, + title: "Add login".into(), + url: "https://github.com/o/r/issues/7".into(), + }); + assert_eq!(issue_cell(&w), "#7"); + } + #[test] fn pr_cell_renders_number_and_state() { let mut w = base(); diff --git a/src/output/table.rs b/src/output/table.rs index 524c24b..2f80861 100644 --- a/src/output/table.rs +++ b/src/output/table.rs @@ -64,6 +64,7 @@ fn cell_color(column: Column, value: &str) -> Option<&'static str> { _ if trimmed.contains("(draft)") => Some(ansi::DIM), _ => None, }, + Column::Issue if !trimmed.is_empty() => Some(ansi::CYAN), _ => None, } } diff --git a/src/worktree/rows.rs b/src/worktree/rows.rs index 808e9dd..43d2192 100644 --- a/src/worktree/rows.rs +++ b/src/worktree/rows.rs @@ -20,7 +20,7 @@ use crate::git::{ CommitInfo, Upstream, abbrev_len, ahead_behind, branch_ref, commit_info, default_branch, enumerate, is_ancestor, local_branches, recent_commits, resolve_hex, status_of, upstream_of, }; -use crate::model::{Commit, MergeState, Pr, PrState, Worktree}; +use crate::model::{Commit, IssueLink, MergeState, Pr, PrState, Worktree}; use crate::slug::slugify; use crate::time::iso8601; @@ -67,6 +67,7 @@ pub(crate) fn enrich_worktree(repo: &Repo, git: &dyn GitCli, abbrev: usize, wt: let meta = wtconfig::read_meta(repo.gix(), &branch); wt.base_ref = meta.base_ref.clone(); wt.pr = build_pr(&meta); + wt.issue = build_issue(&meta); wt.pr_url = meta.pr_url.clone(); // Upstream is read from config and is known even for a missing worktree; // ahead/behind needs the working directory, so it is computed only when @@ -208,6 +209,18 @@ fn compute_merge_state( } /// Builds the PR row from cached `wt.*` metadata, if a PR number is recorded. +/// Builds the [`IssueLink`] from `wt..issue*`, or `None` when the branch +/// has no recorded issue. The number is what makes the link real; title and URL +/// are cached conveniences that default to empty. +fn build_issue(meta: &WtMeta) -> Option { + let number = meta.issue_number?; + Some(IssueLink { + number, + title: meta.issue_title.clone().unwrap_or_default(), + url: meta.issue_url.clone().unwrap_or_default(), + }) +} + fn build_pr(meta: &WtMeta) -> Option { let number = meta.pr_number?; let state = meta @@ -281,6 +294,7 @@ fn enrich_branch_row(repo: &Repo, git: &dyn GitCli, abbrev: usize, row: &mut Wor let meta = wtconfig::read_meta(repo.gix(), &branch); row.base_ref = meta.base_ref.clone(); row.pr = build_pr(&meta); + row.issue = build_issue(&meta); row.pr_url = meta.pr_url.clone(); if let Some(up) = upstream_of(repo.gix(), &branch) { row.upstream = Some(up.display); diff --git a/src/worktree/service.rs b/src/worktree/service.rs index 3c39599..483079f 100644 --- a/src/worktree/service.rs +++ b/src/worktree/service.rs @@ -275,6 +275,14 @@ pub struct MetaUpdate { pub pr_title: Option, /// The cached PR URL. pub pr_url: Option, + /// The linked GitHub issue number (issue #100). + pub issue_number: Option, + /// The cached issue title. + pub issue_title: Option, + /// The cached issue URL. + pub issue_url: Option, + /// The generated implementation brief, persisted for embedders. + pub issue_brief: Option, /// Marks the branch as created by `wt` (spec §10), which is what allows a /// later remove to delete it. There is no un-marking: `false` leaves the /// recorded flag as it is. @@ -304,6 +312,18 @@ pub(crate) fn apply_meta( if let Some(url) = &update.pr_url { wtconfig::write_pr_url(git, root, branch, url)?; } + if let Some(number) = update.issue_number { + wtconfig::write_issue_number(git, root, branch, number)?; + } + if let Some(title) = &update.issue_title { + wtconfig::write_issue_title(git, root, branch, title)?; + } + if let Some(url) = &update.issue_url { + wtconfig::write_issue_url(git, root, branch, url)?; + } + if let Some(brief) = &update.issue_brief { + wtconfig::write_issue_brief(git, root, branch, brief)?; + } if update.created_by_wt { wtconfig::mark_created_by_wt(git, root, branch)?; } From ed4ce70e1cdffb5538436611ac811ea1763a470f Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Fri, 28 Aug 2026 19:21:31 -0400 Subject: [PATCH 3/9] feat(config): add the [agent.generation] profile Gives the generation agent a named profile, so the settings `wt` owns have an obvious home and a place for `wt issue` to read from. `[agent.generation]` is the canonical location. The released flat `agent.model` / `agent.effort` keys keep working as deprecated aliases onto the same two layer fields, so no existing configuration needs migrating and there is no major version bump. Setting a key both ways in one file is an error rather than a last-one-wins race, since table iteration order is not something a user should have to reason about. `[agent.work]` is refused with a pointer to karet. It is a new key, so rejecting it removes nothing; it exists to make the split explicit, because `wt` deliberately keeps generation and hands agent execution to karet. Defaults are unchanged (Claude / Sonnet / Medium) and `wt pr open`'s `--model` / `--effort` flags keep their names, so the AI PR auto-fill behaves exactly as before. `provider` accepts only `claude` today; it exists so the field is forward-compatible when more agents are supported. Claude-Session: https://claude.ai/code/session_014ce9P64RKbVU6kj3GmA1mo --- src/commands/config_cmd.rs | 16 +++- src/commands/pr_open.rs | 12 ++- src/config/mod.rs | 4 +- src/config/parse.rs | 170 ++++++++++++++++++++++++++++++++++--- src/config/schema.rs | 79 ++++++++++++----- 5 files changed, 240 insertions(+), 41 deletions(-) diff --git a/src/commands/config_cmd.rs b/src/commands/config_cmd.rs index ec241dd..1e7a1e6 100644 --- a/src/commands/config_cmd.rs +++ b/src/commands/config_cmd.rs @@ -26,6 +26,9 @@ const KEYS: &[&str] = &[ "submodules.init", "submodules.seed", "create.reflink", + "agent.generation.provider", + "agent.generation.model", + "agent.generation.effort", "agent.model", "agent.effort", "list.show_untracked", @@ -204,6 +207,9 @@ fn key_type(key: &str) -> Option { | "submodules.init" | "submodules.seed" | "create.reflink" + | "agent.generation.provider" + | "agent.generation.model" + | "agent.generation.effort" | "agent.model" | "agent.effort" | "ui.color" @@ -244,8 +250,14 @@ fn config_value(config: &Config, key: &str) -> Result> { "submodules.init" => Some(config.submodules_init.identifier().to_string()), "submodules.seed" => Some(config.submodules_seed.identifier().to_string()), "create.reflink" => Some(config.create_reflink.identifier().to_string()), - "agent.model" => Some(config.agent_model.id().to_string()), - "agent.effort" => Some(config.agent_effort.id().to_string()), + "agent.generation.provider" => Some(config.agent_generation.provider.as_str().to_string()), + // The flat keys remain readable aliases for the same two settings. + "agent.generation.model" | "agent.model" => { + Some(config.agent_generation.model.id().to_string()) + } + "agent.generation.effort" | "agent.effort" => { + Some(config.agent_generation.effort.id().to_string()) + } "list.show_untracked" => Some(config.list_show_untracked.to_string()), "list.columns" => Some( config diff --git a/src/commands/pr_open.rs b/src/commands/pr_open.rs index ae2f774..32f94b7 100644 --- a/src/commands/pr_open.rs +++ b/src/commands/pr_open.rs @@ -317,7 +317,7 @@ pub(crate) fn resolve_agent_options(args: &PrOpenArgs, config: &Config) -> Resul "unknown --model {m:?}; expected one of: opus, sonnet, haiku" )) })?, - None => config.agent_model, + None => config.agent_generation.model, }; let effort = match &args.effort { Some(e) => Effort::parse(e).ok_or_else(|| { @@ -325,7 +325,7 @@ pub(crate) fn resolve_agent_options(args: &PrOpenArgs, config: &Config) -> Resul "unknown --effort {e:?}; expected one of: low, medium, high" )) })?, - None => config.agent_effort, + None => config.agent_generation.effort, }; Ok(AgentOptions { model, effort }) } @@ -451,6 +451,7 @@ pub(crate) fn record_pr_metadata( #[cfg(test)] mod tests { use super::*; + use crate::config::GenerationAgentConfig; use crate::git::cli::RealGit; use crate::git::discover::Repo; use crate::testutil::{FakeAgent, FakeGh, TestRepo}; @@ -756,8 +757,11 @@ mod tests { #[test] fn resolve_agent_options_flags_override_config() { let config = Config { - agent_model: AgentModel::Haiku, - agent_effort: Effort::Low, + agent_generation: GenerationAgentConfig { + model: AgentModel::Haiku, + effort: Effort::Low, + ..GenerationAgentConfig::default() + }, ..Config::default() }; // No flags: config defaults win. diff --git a/src/config/mod.rs b/src/config/mod.rs index 09166ce..fbbc63f 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -17,7 +17,9 @@ use crate::cx::Env; use crate::error::{Error, Result}; pub use parse::parse_layer; -pub use schema::{Config, ConfigLayer, CreateReflink, SubmoduleInit, SubmoduleSeed}; +pub use schema::{ + Config, ConfigLayer, CreateReflink, GenerationAgentConfig, SubmoduleInit, SubmoduleSeed, +}; pub use wtconfig::WtMeta; /// The path to the global `config.toml`, honoring `$XDG_CONFIG_HOME` and falling diff --git a/src/config/parse.rs b/src/config/parse.rs index 2d591b3..75c016a 100644 --- a/src/config/parse.rs +++ b/src/config/parse.rs @@ -7,7 +7,7 @@ use ratatui::style::Color; use toml::Value; -use crate::agent::{AgentModel, Effort}; +use crate::agent::{AgentKind, AgentModel, Effort}; use crate::config::schema::{ConfigLayer, CreateReflink, SubmoduleInit, SubmoduleSeed}; use crate::error::{Error, Result}; #[cfg(feature = "tui")] @@ -168,22 +168,102 @@ fn parse_submodules(file: &str, value: &Value, layer: &mut ConfigLayer) -> Resul Ok(()) } -/// Parses the `[agent]` table, validating the model and effort identifiers. +/// Parses the `[agent]` table. +/// +/// `[agent.generation]` is the canonical home for the generation profile. The +/// flat `agent.model` / `agent.effort` keys are kept as deprecated aliases for +/// the same two settings, so existing configurations keep working; setting a +/// key both ways in one file is rejected rather than silently resolved by +/// table order. +/// +/// `[agent.work]` is refused outright: `wt` does not run coding agents, so +/// there is no work profile to configure here. fn parse_agent(file: &str, value: &Value, layer: &mut ConfigLayer) -> Result<()> { + let mut flat_model = false; + let mut flat_effort = false; + let mut generation_model = false; + let mut generation_effort = false; + for (sub, val) in as_table(file, "agent", value)? { let key = format!("agent.{sub}"); match sub.as_str() { "model" => { - let text = as_string(file, &key, val)?; - let model = AgentModel::parse(&text) - .ok_or_else(|| cfg_err(file, &key, "expected one of: opus, sonnet, haiku"))?; - layer.agent_model = Some(model); + layer.agent_generation_model = Some(parse_agent_model(file, &key, val)?); + flat_model = true; + tracing::warn!("{file}: `agent.model` is deprecated; use `agent.generation.model`"); } "effort" => { + layer.agent_generation_effort = Some(parse_agent_effort(file, &key, val)?); + flat_effort = true; + tracing::warn!( + "{file}: `agent.effort` is deprecated; use `agent.generation.effort`" + ); + } + "generation" => { + parse_agent_generation( + file, + val, + layer, + &mut generation_model, + &mut generation_effort, + )?; + } + "work" => { + return Err(cfg_err( + file, + &key, + "`wt` does not run coding agents, so it has no work profile; \ + agent execution belongs to karet (https://github.com/getkono/karet). \ + `wt` owns only `[agent.generation]`", + )); + } + _ => return Err(cfg_err(file, &key, "unknown configuration key")), + } + } + + // Order within a table is not something a user should have to reason about, + // so a key set both ways is an error rather than a last-one-wins race. + if flat_model && generation_model { + return Err(cfg_err( + file, + "agent.model", + "conflicts with `agent.generation.model` in the same file; set only one", + )); + } + if flat_effort && generation_effort { + return Err(cfg_err( + file, + "agent.effort", + "conflicts with `agent.generation.effort` in the same file; set only one", + )); + } + Ok(()) +} + +/// Parses the `[agent.generation]` sub-table. +fn parse_agent_generation( + file: &str, + value: &Value, + layer: &mut ConfigLayer, + saw_model: &mut bool, + saw_effort: &mut bool, +) -> Result<()> { + for (sub, val) in as_table(file, "agent.generation", value)? { + let key = format!("agent.generation.{sub}"); + match sub.as_str() { + "provider" => { let text = as_string(file, &key, val)?; - let effort = Effort::parse(&text) - .ok_or_else(|| cfg_err(file, &key, "expected one of: low, medium, high"))?; - layer.agent_effort = Some(effort); + let provider = AgentKind::parse(&text) + .ok_or_else(|| cfg_err(file, &key, "expected one of: claude"))?; + layer.agent_generation_provider = Some(provider); + } + "model" => { + layer.agent_generation_model = Some(parse_agent_model(file, &key, val)?); + *saw_model = true; + } + "effort" => { + layer.agent_generation_effort = Some(parse_agent_effort(file, &key, val)?); + *saw_effort = true; } _ => return Err(cfg_err(file, &key, "unknown configuration key")), } @@ -191,6 +271,20 @@ fn parse_agent(file: &str, value: &Value, layer: &mut ConfigLayer) -> Result<()> Ok(()) } +/// Validates a model identifier, shared by the canonical key and its alias so +/// the two cannot drift. +fn parse_agent_model(file: &str, key: &str, value: &Value) -> Result { + let text = as_string(file, key, value)?; + AgentModel::parse(&text) + .ok_or_else(|| cfg_err(file, key, "expected one of: opus, sonnet, haiku")) +} + +/// Validates an effort identifier, shared by the canonical key and its alias. +fn parse_agent_effort(file: &str, key: &str, value: &Value) -> Result { + let text = as_string(file, key, value)?; + Effort::parse(&text).ok_or_else(|| cfg_err(file, key, "expected one of: low, medium, high")) +} + /// Parses the `[list]` table, validating column identifiers. fn parse_list(file: &str, value: &Value, layer: &mut ConfigLayer) -> Result<()> { for (sub, val) in as_table(file, "list", value)? { @@ -375,8 +469,8 @@ mod tests { assert_eq!(layer.remove_untracked_blocks, Some(true)); assert_eq!(layer.pr_default_remote.as_deref(), Some("upstream")); assert_eq!(layer.submodules_init, Some(SubmoduleInit::Always)); - assert_eq!(layer.agent_model, Some(AgentModel::Opus)); - assert_eq!(layer.agent_effort, Some(Effort::High)); + assert_eq!(layer.agent_generation_model, Some(AgentModel::Opus)); + assert_eq!(layer.agent_generation_effort, Some(Effort::High)); assert_eq!(layer.list_show_untracked, Some(false)); assert_eq!(layer.list_columns, Some(vec![Column::Branch, Column::Pr])); assert_eq!(layer.ui_nerd_fonts, Some(true)); @@ -457,6 +551,60 @@ mod tests { assert_eq!(key, "agent.wiggle"); } + #[test] + fn generation_profile_parses_every_key() { + let layer = + parse("[agent.generation]\nprovider = \"claude\"\nmodel = \"opus\"\neffort = \"high\"") + .unwrap(); + assert_eq!(layer.agent_generation_provider, Some(AgentKind::Claude)); + assert_eq!(layer.agent_generation_model, Some(AgentModel::Opus)); + assert_eq!(layer.agent_generation_effort, Some(Effort::High)); + } + + #[test] + fn flat_agent_keys_are_aliases_for_the_generation_profile() { + // The released flat keys keep working and land on exactly the same + // layer fields, so an existing config needs no migration. + let layer = parse("[agent]\nmodel = \"opus\"\neffort = \"high\"").unwrap(); + assert_eq!(layer.agent_generation_model, Some(AgentModel::Opus)); + assert_eq!(layer.agent_generation_effort, Some(Effort::High)); + } + + #[test] + fn a_key_set_both_ways_in_one_file_is_rejected() { + // Table iteration order is not something a user should have to reason + // about, so this is an error rather than a last-one-wins race. + let (key, reason) = config_reason( + parse("[agent]\nmodel = \"opus\"\n[agent.generation]\nmodel = \"haiku\"").unwrap_err(), + ); + assert_eq!(key, "agent.model"); + assert!(reason.contains("agent.generation.model"), "{reason}"); + let (key, reason) = config_reason( + parse("[agent]\neffort = \"low\"\n[agent.generation]\neffort = \"high\"").unwrap_err(), + ); + assert_eq!(key, "agent.effort"); + assert!(reason.contains("agent.generation.effort"), "{reason}"); + } + + #[test] + fn agent_work_is_rejected_with_a_pointer_to_karet() { + // `wt` keeps the generation profile; running the agent is karet's job. + let (key, reason) = + config_reason(parse("[agent.work]\nprovider = \"claude\"").unwrap_err()); + assert_eq!(key, "agent.work"); + assert!(reason.contains("karet"), "{reason}"); + } + + #[test] + fn unknown_generation_keys_are_rejected_with_the_dotted_path() { + let (key, _) = config_reason(parse("[agent.generation]\nwiggle = true").unwrap_err()); + assert_eq!(key, "agent.generation.wiggle"); + let (key, reason) = + config_reason(parse("[agent.generation]\nprovider = \"codex\"").unwrap_err()); + assert_eq!(key, "agent.generation.provider"); + assert!(reason.contains("claude"), "{reason}"); + } + #[test] fn submodules_init_parses_and_validates() { assert_eq!( diff --git a/src/config/schema.rs b/src/config/schema.rs index f9d7836..8dc1c17 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -4,7 +4,7 @@ #[cfg(feature = "tui")] use ratatui::style::Color; -use crate::agent::{AgentModel, Effort}; +use crate::agent::{AgentKind, AgentModel, Effort}; use crate::cx::Env; #[cfg(feature = "tui")] use crate::keys::{KeyAction, KeyChord, Keymap}; @@ -147,6 +147,35 @@ impl CreateReflink { } } +/// The generation agent profile (`[agent.generation]`): which agent `wt` drives +/// for the short, structured generation steps it owns — the PR auto-fill and +/// the `wt issue` branch/brief proposal. +/// +/// `wt` owns *generation* only. Running a coding agent on the work itself is +/// karet's job, which is why there is no work profile here and why +/// `[agent.work]` is rejected with a pointer to karet. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct GenerationAgentConfig { + /// Which agent CLI to drive. + pub provider: AgentKind, + /// The model to request. + pub model: AgentModel, + /// The reasoning effort to request. + pub effort: Effort, +} + +impl Default for GenerationAgentConfig { + fn default() -> Self { + // Unchanged from the flat `[agent]` defaults these keys replace, so + // `wt pr open --ai` behaves identically before and after the move. + GenerationAgentConfig { + provider: AgentKind::Claude, + model: AgentModel::default(), + effort: Effort::default(), + } + } +} + /// The fully-resolved configuration after merging all layers. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Config { @@ -177,12 +206,11 @@ pub struct Config { /// Whether to copy-on-write clone a new worktree's content from an existing /// worktree instead of checking it out. pub create_reflink: CreateReflink, - /// Default model for the AI PR auto-fill (`wt pr open --ai`); overridable - /// per-invocation by `--model` or the TUI's `Ctrl-M` key. - pub agent_model: AgentModel, - /// Default effort for the AI PR auto-fill; overridable by `--effort` or the - /// TUI's `Ctrl-E` key. - pub agent_effort: Effort, + /// The generation agent profile (`[agent.generation]`), used by the AI PR + /// auto-fill (`wt pr open --ai`) and by `wt issue`. Model and effort are + /// overridable per-invocation by `--model`/`--effort` or the TUI's + /// `Ctrl-M`/`Ctrl-E` keys. + pub agent_generation: GenerationAgentConfig, /// Show `?` in the dirty column for untracked files. pub list_show_untracked: bool, /// Ordered list of columns to display in `wt list`. @@ -219,8 +247,7 @@ impl Default for Config { submodules_init: SubmoduleInit::default(), submodules_seed: SubmoduleSeed::default(), create_reflink: CreateReflink::default(), - agent_model: AgentModel::default(), - agent_effort: Effort::default(), + agent_generation: GenerationAgentConfig::default(), list_show_untracked: true, list_columns: Column::ALL.to_vec(), ui_nerd_fonts: false, @@ -279,11 +306,14 @@ impl Config { if let Some(v) = layer.create_reflink { self.create_reflink = v; } - if let Some(v) = layer.agent_model { - self.agent_model = v; + if let Some(v) = layer.agent_generation_provider { + self.agent_generation.provider = v; + } + if let Some(v) = layer.agent_generation_model { + self.agent_generation.model = v; } - if let Some(v) = layer.agent_effort { - self.agent_effort = v; + if let Some(v) = layer.agent_generation_effort { + self.agent_generation.effort = v; } if let Some(v) = layer.list_show_untracked { self.list_show_untracked = v; @@ -370,10 +400,12 @@ pub struct ConfigLayer { pub submodules_seed: Option, /// `create.reflink`. pub create_reflink: Option, - /// `agent.model`. - pub agent_model: Option, - /// `agent.effort`. - pub agent_effort: Option, + /// `agent.generation.provider`. + pub agent_generation_provider: Option, + /// `agent.generation.model`, or its deprecated `agent.model` alias. + pub agent_generation_model: Option, + /// `agent.generation.effort`, or its deprecated `agent.effort` alias. + pub agent_generation_effort: Option, /// `list.show_untracked`. pub list_show_untracked: Option, /// `list.columns`. @@ -489,8 +521,9 @@ mod tests { assert!(!c.remove_untracked_blocks); assert_eq!(c.pr_default_remote, "origin"); assert_eq!(c.submodules_init, SubmoduleInit::Prompt); - assert_eq!(c.agent_model, AgentModel::Sonnet); - assert_eq!(c.agent_effort, Effort::Medium); + assert_eq!(c.agent_generation.provider, AgentKind::Claude); + assert_eq!(c.agent_generation.model, AgentModel::Sonnet); + assert_eq!(c.agent_generation.effort, Effort::Medium); assert!(c.list_show_untracked); assert_eq!(c.list_columns, Column::ALL.to_vec()); assert!(!c.ui_nerd_fonts); @@ -542,8 +575,8 @@ mod tests { remove_delete_merged_branch: Some(false), remove_untracked_blocks: Some(true), submodules_init: Some(SubmoduleInit::Always), - agent_model: Some(AgentModel::Haiku), - agent_effort: Some(Effort::Low), + agent_generation_model: Some(AgentModel::Haiku), + agent_generation_effort: Some(Effort::Low), list_show_untracked: Some(false), ui_nerd_fonts: Some(true), ui_color: Some(ColorChoice::Never), @@ -557,8 +590,8 @@ mod tests { assert!(!c.remove_delete_merged_branch); assert!(c.remove_untracked_blocks); assert_eq!(c.submodules_init, SubmoduleInit::Always); - assert_eq!(c.agent_model, AgentModel::Haiku); - assert_eq!(c.agent_effort, Effort::Low); + assert_eq!(c.agent_generation.model, AgentModel::Haiku); + assert_eq!(c.agent_generation.effort, Effort::Low); assert!(!c.list_show_untracked); assert!(c.ui_nerd_fonts); assert_eq!(c.ui_color, ColorChoice::Never); From 7415d9e14460bbc4b4ea95b676c39e12f39fc38c Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Fri, 28 Aug 2026 20:25:15 -0400 Subject: [PATCH 4/9] feat(agent): bound a generation run with a deadline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Command::output()` waits forever, so an agent that hangs hangs `wt`. That is tolerable for a PR draft the user is watching, but not for `wt issue`, where generation is best-effort and must never be able to block worktree creation. Adds `AgentOptions::timeout`. `None` waits indefinitely and remains the default, so the PR auto-fill and version detection behave exactly as before; both existing call sites pass it explicitly. The deadline cannot be expressed with `output()`, which blocks until the pipes close, so the timeout path spawns the child, drains stdout and stderr on their own threads, and polls `try_wait` until the deadline. Both pipes are drained concurrently because a child that fills the stderr buffer would otherwise block forever with only stdout being read. Killing the child closes its pipes, which is what lets the reader threads finish. `AgentOptions` stays `Copy` — `Option` is `Copy`, and the TUI compose state and `pr_open`'s assertions depend on that. Tested against a real subprocess rather than a fake: `sh -c "sleep 30"` with a 100ms deadline returns a timeout in well under a second, and the deadline path is also shown to return stdout intact and to still map a non-zero exit (with its stderr) to `Error::Subprocess`. Claude-Session: https://claude.ai/code/session_014ce9P64RKbVU6kj3GmA1mo --- src/agent/mod.rs | 187 ++++++++++++++++++++++++++++++++----- src/agent/model.rs | 11 ++- src/commands/pr_open.rs | 9 +- src/error.rs | 11 +++ src/testutil.rs | 12 +++ src/tui/runtime/effects.rs | 1 + src/tui/runtime/mod.rs | 1 + 7 files changed, 209 insertions(+), 23 deletions(-) diff --git a/src/agent/mod.rs b/src/agent/mod.rs index a9c41e5..a302db3 100644 --- a/src/agent/mod.rs +++ b/src/agent/mod.rs @@ -11,8 +11,10 @@ pub mod model; pub mod spec; pub mod types; +use std::io::Read; use std::path::Path; -use std::process::Command; +use std::process::{Command, ExitStatus, Stdio}; +use std::time::{Duration, Instant}; use crate::error::{Error, Result}; pub use model::{AgentModel, AgentOptions, Effort}; @@ -70,7 +72,7 @@ impl AgentClient for RealAgent { /// [`RealAgent::detect`] so tests can drive every branch with a stand-in /// binary. A missing binary maps to `Ok(None)`; other failures propagate. fn detect_with(binary: &str, kind: AgentKind, spec: &AgentSpec) -> Result> { - match run_agent(binary, None, &spec::version_argv(spec)) { + match run_agent(binary, None, &spec::version_argv(spec), None) { Ok(stdout) => Ok(Some(DetectedAgent { kind, binary: binary.to_string(), @@ -93,38 +95,114 @@ fn run_with( ) -> Result { let prompt = spec::apply_effort(opts.effort, prompt); let argv = spec::prompt_argv(spec, &prompt, opts.model); - let stdout = run_agent(binary, Some(dir), &argv)?; + let stdout = run_agent(binary, Some(dir), &argv, opts.timeout)?; spec::parse_result(kind, spec.result_format, &stdout) } /// Runs an agent `binary` (optionally in `dir`), mapping a missing binary to /// [`Error::AgentUnavailable`] and a non-zero exit to [`Error::Subprocess`]. /// Mirrors `gh`'s `run_gh` helper. -fn run_agent(binary: &str, dir: Option<&Path>, args: &[String]) -> Result { +/// +/// With `timeout` set, the child is killed and [`Error::AgentTimeout`] returned +/// once the deadline passes. `None` waits indefinitely — the historical +/// behaviour, kept for version detection and for callers that pass no deadline. +fn run_agent( + binary: &str, + dir: Option<&Path>, + args: &[String], + timeout: Option, +) -> Result { let mut cmd = Command::new(binary); if let Some(dir) = dir { cmd.current_dir(dir); } cmd.args(args); - let output = match cmd.output() { - Ok(output) => output, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - return Err(Error::AgentUnavailable(format!( - "{binary} is not installed or not on PATH" - ))); + + let Some(limit) = timeout else { + return match cmd.output() { + Ok(output) => finish(binary, output.status, &output.stdout, &output.stderr), + Err(e) => Err(spawn_error(binary, &e)), + }; + }; + + // `Command::output()` blocks until the pipes close, so it cannot be given a + // deadline. Read the pipes on their own threads and keep the `Child` here, + // so this thread still owns the handle it needs to kill. + cmd.stdout(Stdio::piped()).stderr(Stdio::piped()); + let mut child = match cmd.spawn() { + Ok(child) => child, + Err(e) => return Err(spawn_error(binary, &e)), + }; + let mut out_pipe = child.stdout.take(); + let mut err_pipe = child.stderr.take(); + // Draining both pipes concurrently matters: a child that fills the stderr + // pipe buffer blocks forever if only stdout is being read. + let out_reader = std::thread::spawn(move || read_pipe(out_pipe.as_mut())); + let err_reader = std::thread::spawn(move || read_pipe(err_pipe.as_mut())); + + let deadline = Instant::now() + limit; + let status = loop { + match child.try_wait() { + Ok(Some(status)) => break Some(status), + Ok(None) => {} + Err(e) => return Err(spawn_error(binary, &e)), } - Err(e) => { - return Err(Error::AgentUnavailable(format!( - "failed to run {binary}: {e}" - ))); + if Instant::now() >= deadline { + // Killing closes the child's pipes, which is what lets the reader + // threads below finish rather than block forever. + let _ = child.kill(); + let _ = child.wait(); + break None; } + std::thread::sleep(POLL_INTERVAL); }; - if output.status.success() { - return Ok(String::from_utf8_lossy(&output.stdout).into_owned()); + + // A panicking reader yields no output rather than poisoning the run. + let stdout = out_reader.join().unwrap_or_default(); + let stderr = err_reader.join().unwrap_or_default(); + + match status { + Some(status) => finish(binary, status, &stdout, &stderr), + None => Err(Error::AgentTimeout { + binary: binary.to_string(), + // Sub-second deadlines still report `1s`; the message is for humans, + // and "did not respond within 0s" reads as a bug. + seconds: limit.as_secs().max(1), + }), + } +} + +/// How often the deadline loop checks whether the child has exited. Short +/// enough that a fast agent is not held up perceptibly, long enough not to spin. +const POLL_INTERVAL: Duration = Duration::from_millis(10); + +/// Drains a child pipe to end, yielding empty bytes if it is absent or fails. +fn read_pipe(pipe: Option<&mut impl Read>) -> Vec { + let mut buf = Vec::new(); + if let Some(pipe) = pipe { + let _ = pipe.read_to_end(&mut buf); + } + buf +} + +/// Maps a spawn failure to [`Error::AgentUnavailable`], distinguishing a missing +/// binary from any other launch failure. +fn spawn_error(binary: &str, e: &std::io::Error) -> Error { + if e.kind() == std::io::ErrorKind::NotFound { + Error::AgentUnavailable(format!("{binary} is not installed or not on PATH")) + } else { + Error::AgentUnavailable(format!("failed to run {binary}: {e}")) + } +} + +/// Maps a finished process to its stdout, or to [`Error::Subprocess`]. +fn finish(binary: &str, status: ExitStatus, stdout: &[u8], stderr: &[u8]) -> Result { + if status.success() { + return Ok(String::from_utf8_lossy(stdout).into_owned()); } Err(Error::Subprocess { program: binary.to_string(), - stderr: String::from_utf8_lossy(&output.stderr).trim().to_string(), + stderr: String::from_utf8_lossy(stderr).trim().to_string(), }) } @@ -205,7 +283,7 @@ mod tests { #[test] fn run_agent_maps_missing_binary_to_unavailable() { - let err = run_agent(MISSING, None, &["--version".to_string()]).unwrap_err(); + let err = run_agent(MISSING, None, &["--version".to_string()], None).unwrap_err(); assert!(matches!(err, Error::AgentUnavailable(_))); } @@ -254,20 +332,87 @@ mod tests { #[test] fn run_agent_returns_stdout_on_success() { - let out = - run_agent("sh", None, &["-c".to_string(), "printf hello".to_string()]).unwrap(); + let out = run_agent( + "sh", + None, + &["-c".to_string(), "printf hello".to_string()], + None, + ) + .unwrap(); assert_eq!(out, "hello"); } #[test] fn run_agent_maps_nonzero_exit_to_subprocess() { - let err = run_agent("sh", None, &["-c".to_string(), "exit 3".to_string()]).unwrap_err(); + let err = + run_agent("sh", None, &["-c".to_string(), "exit 3".to_string()], None).unwrap_err(); match err { Error::Subprocess { program, .. } => assert_eq!(program, "sh"), other => panic!("expected subprocess error, got {other:?}"), } } + #[test] + fn run_agent_kills_a_child_that_outlives_its_deadline() { + // The real defect this guards: `Command::output()` waits forever, so + // an agent that hangs used to hang `wt`. Sleeping far longer than the + // deadline proves the deadline — not the sleep — is what ends the run. + let started = Instant::now(); + let err = run_agent( + "sh", + None, + &["-c".to_string(), "sleep 30".to_string()], + Some(Duration::from_millis(100)), + ) + .unwrap_err(); + let elapsed = started.elapsed(); + match err { + Error::AgentTimeout { binary, seconds } => { + assert_eq!(binary, "sh"); + // Sub-second deadlines still report a whole second. + assert_eq!(seconds, 1); + } + other => panic!("expected a timeout, got {other:?}"), + } + assert!( + elapsed < Duration::from_secs(10), + "returned after {elapsed:?}; the child was not killed" + ); + } + + #[test] + fn a_deadline_does_not_disturb_a_process_that_finishes() { + // The deadline path reads the pipes on separate threads, so prove it + // still returns stdout intact rather than only working on timeout. + let out = run_agent( + "sh", + None, + &["-c".to_string(), "printf hello".to_string()], + Some(Duration::from_secs(30)), + ) + .unwrap(); + assert_eq!(out, "hello"); + } + + #[test] + fn a_deadline_still_maps_a_nonzero_exit_to_subprocess() { + let err = run_agent( + "sh", + None, + &["-c".to_string(), "printf oops >&2; exit 3".to_string()], + Some(Duration::from_secs(30)), + ) + .unwrap_err(); + match err { + Error::Subprocess { program, stderr } => { + assert_eq!(program, "sh"); + // Proves stderr is drained too, not just stdout. + assert_eq!(stderr, "oops"); + } + other => panic!("expected subprocess error, got {other:?}"), + } + } + #[test] fn detect_with_parses_version_from_real_process() { let detected = detect_with("sh", AgentKind::Claude, &SH_VERSION) diff --git a/src/agent/model.rs b/src/agent/model.rs index ada4a56..ebc2e3d 100644 --- a/src/agent/model.rs +++ b/src/agent/model.rs @@ -6,6 +6,8 @@ //! drive the config layer, the CLI flags, and the TUI's live cycle keys without //! any process or I/O. +use std::time::Duration; + use serde::Serialize; /// A selectable model tier for a code agent. The variants currently encode the @@ -159,13 +161,20 @@ impl Effort { } } -/// The model and effort selected for a single agent run. +/// The model, effort, and deadline selected for a single agent run. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub struct AgentOptions { /// The model tier to drive. pub model: AgentModel, /// How much effort to spend. pub effort: Effort, + /// How long to wait before killing the agent. `None` waits indefinitely, + /// which is the historical behaviour and remains the default. + /// + /// Deliberately a [`Duration`] rather than anything owned: `AgentOptions` is + /// `Copy`, and the compose form, the TUI and `pr_open`'s tests all rely on + /// that. + pub timeout: Option, } #[cfg(test)] diff --git a/src/commands/pr_open.rs b/src/commands/pr_open.rs index 32f94b7..0f2f844 100644 --- a/src/commands/pr_open.rs +++ b/src/commands/pr_open.rs @@ -327,7 +327,13 @@ pub(crate) fn resolve_agent_options(args: &PrOpenArgs, config: &Config) -> Resul })?, None => config.agent_generation.effort, }; - Ok(AgentOptions { model, effort }) + Ok(AgentOptions { + model, + effort, + // The PR auto-fill has always waited indefinitely; `wt issue` is what + // introduces a deadline, and only for its own generation step. + timeout: None, + }) } /// Runs the code agent (`claude`) to draft a PR `(title, body)` from `ctx` with @@ -721,6 +727,7 @@ mod tests { let opts = AgentOptions { model: AgentModel::Opus, effort: Effort::High, + timeout: None, }; draft_with_ai(&agent, &ctx_for("feat", "main", false), dir.path(), &opts).unwrap(); assert_eq!(agent.last_opts(), Some(opts)); diff --git a/src/error.rs b/src/error.rs index fbf8d2d..a233986 100644 --- a/src/error.rs +++ b/src/error.rs @@ -73,6 +73,17 @@ pub enum Error { #[error("{0}")] AgentUnavailable(String), + /// A code-agent run exceeded its deadline and was killed. Generation is + /// always best-effort, so callers are expected to fall back rather than + /// surface this to the user as a failure. + #[error("{binary} did not respond within {seconds}s")] + AgentTimeout { + /// The agent binary that was killed. + binary: String, + /// The deadline it exceeded, in seconds. + seconds: u64, + }, + /// The repository's `wt.schema` was written by a newer `wt` (or embedder) /// than this one; reading it could silently misinterpret metadata, so the /// operation is refused (issue #99). diff --git a/src/testutil.rs b/src/testutil.rs index 23efeb7..6da8340 100644 --- a/src/testutil.rs +++ b/src/testutil.rs @@ -193,6 +193,9 @@ pub(crate) enum AgentBehavior { Erroring(String), /// Agent absent: `detect` returns `Ok(None)` and `run` returns `AgentUnavailable`. Unavailable, + /// Agent present but unresponsive: `run` returns `AgentTimeout`, as the real + /// client does once a run exceeds its deadline. + TimingOut, } /// A fake [`AgentClient`] for tests: returns a canned draft, simulates an @@ -227,6 +230,11 @@ impl FakeAgent { FakeAgent::new(AgentBehavior::Unavailable) } + /// A present but unresponsive agent (`run` → `AgentTimeout`). + pub(crate) fn timing_out() -> Self { + FakeAgent::new(AgentBehavior::TimingOut) + } + /// The [`AgentOptions`] passed to the most recent `run`, if any. pub(crate) fn last_opts(&self) -> Option { *self.last_opts.lock().expect("lock") @@ -270,6 +278,10 @@ impl AgentClient for FakeAgent { raw: serde_json::Value::Null, }), AgentBehavior::Unavailable => Err(Error::AgentUnavailable("claude unavailable".into())), + AgentBehavior::TimingOut => Err(Error::AgentTimeout { + binary: kind.as_str().to_string(), + seconds: 120, + }), } } } diff --git a/src/tui/runtime/effects.rs b/src/tui/runtime/effects.rs index 9ea150e..33080eb 100644 --- a/src/tui/runtime/effects.rs +++ b/src/tui/runtime/effects.rs @@ -756,6 +756,7 @@ pub(crate) fn do_draft_pr_ai( Mode::PrCompose(state) => crate::agent::AgentOptions { model: state.model, effort: state.effort, + timeout: None, }, _ => crate::agent::AgentOptions::default(), }; diff --git a/src/tui/runtime/mod.rs b/src/tui/runtime/mod.rs index 137a558..3294b79 100644 --- a/src/tui/runtime/mod.rs +++ b/src/tui/runtime/mod.rs @@ -1734,6 +1734,7 @@ mod tests { Some(crate::agent::AgentOptions { model: crate::agent::AgentModel::Opus, effort: crate::agent::Effort::High, + timeout: None, }) ); } From 2dfa888bccd735cf28cd606b76733064c5c4462c Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Fri, 28 Aug 2026 20:33:31 -0400 Subject: [PATCH 5/9] feat(cli): add wt issue to create an issue-linked worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wt issue ` fetches a GitHub issue, proposes a conventional branch name and an implementation brief, creates (or reuses) the worktree, and records the link — then stops. It does not launch a coding agent: running the work belongs to karet over ACP (issue #100). Generation is best-effort, and that is the point of issue #98. A model that returns nonsense, an agent that is not installed, and an agent that hangs all degrade to a deterministic fallback branch derived from the issue's own labels, then its type, then `feat`. A malformed branch keeps the generated brief, since the two are independent; a failed call leaves the brief empty. The only thing that still fails is a bad --model value, which is the user's to fix. Worktree creation never depends on model behaviour. The `TYPE/{number}-SLUG` contract comes from `wt::naming` (issue #96), which until now had no callers: the prompt fragment the model is given and the validator its answer is checked against are the same code, so they cannot drift. The contract binds the model, not the user. After the review prompts, an edited branch only has to be a legal git branch — the issue link lives in `wt..issueNumber`, never in the branch name, so an off-contract name still resolves back to its issue. It gets a note, not an error. Creation goes through the worktree service rather than shelling into `wt new`, so the confirmation preview and the real target come from one `preview_target` helper. Recording the link is a locked read-check-write against a *freshly discovered* repository: `create_in` writes metadata through the git subprocess, but gix snapshots config at open, so the session handle is stale. It also cannot use `Workspace::write_meta`, which takes the lock itself and so cannot enclose the preceding read — the advisory lock is not reentrant. The reporting tail `wt new` had is extracted to `report_created` so both commands report copy outcomes and hook warnings identically. `default_base_ref` moves from the TUI re-export group to the CLI one, since a CLI command now resolves its base from `origin/HEAD`. Deliberately not included: agent execution in any form, the TUI issue surface, `--json`, `--start`, and the base-staleness pre-flight. Claude-Session: https://claude.ai/code/session_014ce9P64RKbVU6kj3GmA1mo --- src/cli.rs | 57 +++ src/commands/issue.rs | 962 ++++++++++++++++++++++++++++++++++++++++ src/commands/mod.rs | 26 +- src/commands/new.rs | 18 +- src/git/mod.rs | 8 +- src/lib.rs | 3 + src/progress.rs | 96 ++++ src/worktree/mod.rs | 4 +- src/worktree/service.rs | 14 + 9 files changed, 1166 insertions(+), 22 deletions(-) create mode 100644 src/commands/issue.rs create mode 100644 src/progress.rs diff --git a/src/cli.rs b/src/cli.rs index 1c7344d..dc9e7f3 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -85,6 +85,8 @@ pub(crate) enum Command { /// Check out a GitHub PR into its own worktree. #[command(args_conflicts_with_subcommands = true)] Pr(PrArgs), + /// Create a worktree for a GitHub issue, with a generated branch and brief. + Issue(IssueArgs), /// Detailed status for one or all worktrees. Status(StatusArgs), /// Print the absolute path of a matching worktree. @@ -250,6 +252,57 @@ fn submodule_override(init: bool, no_init: bool) -> Option { } } +/// Arguments for `wt issue`. +/// +/// `wt` proposes a branch and brief and creates the worktree; it does not run a +/// coding agent, so there are no agent-execution flags here (issue #100). The +/// `--model`/`--effort` pair tunes only the short generation step, and matches +/// `wt pr open`'s flags because both drive the same `[agent.generation]` +/// profile. +#[derive(Debug, Clone, Args)] +pub(crate) struct IssueArgs { + /// GitHub issue number or URL. + pub(crate) target: String, + /// Use this branch instead of the generated one. + #[arg(long, value_name = "NAME")] + pub(crate) branch: Option, + /// Base ref for a new branch (default: `origin/HEAD`, then the repo default). + #[arg(long, value_name = "REF")] + pub(crate) from: Option, + /// Use this brief instead of the generated one. + #[arg(long, value_name = "TEXT")] + pub(crate) brief: Option, + /// Model for generating the branch and brief (overrides `agent.generation.model`). + #[arg(long, value_name = "MODEL")] + pub(crate) model: Option, + /// Effort for generating the branch and brief (overrides `agent.generation.effort`). + #[arg(long, value_name = "LEVEL")] + pub(crate) effort: Option, + /// Do not switch into the new worktree. + #[arg(long = "no-switch")] + pub(crate) no_switch: bool, + /// Skip the post_create hook. + #[arg(long = "no-hooks")] + pub(crate) no_hooks: bool, + /// Worktree to copy configured files from (default: the current one). + #[arg(long = "copy-from", value_name = "QUERY")] + pub(crate) copy_from: Option, + /// Initialize git submodules after creating the worktree. + #[arg(long = "init-submodules", conflicts_with = "no_init_submodules")] + pub(crate) init_submodules: bool, + /// Do not initialize git submodules. + #[arg(long = "no-init-submodules")] + pub(crate) no_init_submodules: bool, +} + +impl IssueArgs { + /// Resolves the submodule-init flags to an override for the config policy + /// (see [`NewArgs::submodule_override`]). + pub(crate) fn submodule_override(&self) -> Option { + submodule_override(self.init_submodules, self.no_init_submodules) + } +} + /// Arguments for `wt list`. #[derive(Debug, Args)] pub(crate) struct ListArgs { @@ -505,6 +558,7 @@ impl Cli { Some(Command::Drop(_)) => "drop", Some(Command::Prune(_)) => "prune", Some(Command::Pr(_)) => "pr", + Some(Command::Issue(_)) => "issue", Some(Command::Status(_)) => "status", Some(Command::Path(_)) => "path", Some(Command::Root) => "root", @@ -608,6 +662,9 @@ fn route(cli: Cli, cx: &mut Cx) -> Result { Some(Command::Pr(args)) => { crate::commands::pr::run(cx, &crate::hooks::RealHookRunner, &args, json) } + Some(Command::Issue(args)) => { + crate::commands::issue::run(cx, &crate::hooks::RealHookRunner, &args) + } Some(Command::Status(args)) => crate::commands::status_cmd::run(cx, &args, json), Some(Command::Path(args)) => crate::commands::path::run(cx, &args), Some(Command::Root) => crate::commands::root::run(cx), diff --git a/src/commands/issue.rs b/src/commands/issue.rs new file mode 100644 index 0000000..625a441 --- /dev/null +++ b/src/commands/issue.rs @@ -0,0 +1,962 @@ +//! `wt issue` — create (or reuse) a worktree for a GitHub issue, with a branch +//! name and implementation brief proposed by the generation agent. +//! +//! `wt` deliberately stops here: it fetches the issue, proposes a branch and +//! brief, creates the worktree, and records the link. It does **not** launch a +//! coding agent — running the work is karet's job over the Agent Client +//! Protocol (issue #100). What this command hands back is a prepared worktree +//! and the metadata to find it again. +//! +//! Generation is best-effort throughout. A model that returns nonsense, an agent +//! that is not installed, and an agent that hangs all degrade to a deterministic +//! fallback branch built from the issue's own labels and title (issue #98), so +//! **worktree creation never depends on model behaviour**. +//! +//! The `TYPE/{number}-SLUG` contract itself lives in [`crate::naming`], which is +//! pure and shared: the prompt fragment the model is given and the validator its +//! answer is checked against come from the same place and cannot drift +//! (issue #96). + +use crate::agent::{AgentClient, AgentKind, AgentModel, AgentOptions, Effort}; +use crate::cli::IssueArgs; +use crate::commands::{Session, confirm, finish_worktree, open_session, report_created}; +use crate::config::{Config, wtconfig}; +use crate::cx::Cx; +use crate::error::{Error, Result}; +use crate::gh::IssueView; +use crate::git::cli::GitCli; +use crate::git::discover::Repo; +use crate::git::{all_branches, default_base_ref, local_branches, validate_branch_name}; +use crate::hooks::{HookContext, HookRunner}; +use crate::naming::{self, BranchKind}; +use crate::progress; +use crate::worktree::{ + CreateOptions, MetaUpdate, apply_meta, create_in, lock_repo, preview_target, +}; + +use super::Nav; + +/// How long the generation agent gets before it is killed and the fallback is +/// used. Generous enough for a short structured answer, short enough that a +/// hung agent does not hold up a worktree. +const GENERATION_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(120); + +/// The branch name and implementation brief for an issue worktree. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct IssuePlan { + /// The branch to create or reuse. + pub(crate) branch: String, + /// The implementation brief; empty when generation produced none. + pub(crate) brief: String, +} + +/// Runs `wt issue`. +pub(crate) fn run(cx: &mut Cx, hooks: &dyn HookRunner, args: &IssueArgs) -> Result { + // Gate before anything costly. `wt issue` reviews values with the user, so + // without a terminal it needs `--yes` — and finding that out *after* a + // network round trip and an LLM call would be rude and slow. + if !cx.err.is_tty() && !cx.assume_yes { + return Err(Error::usage( + "`wt issue` requires an interactive terminal or `--yes`", + )); + } + + let git = cx.git.clone(); + let session = open_session(cx, git.as_ref())?; + let dir = session + .repo + .current_workdir() + .unwrap_or_else(|| session.primary_root.clone()); + + let gh = cx.gh.clone(); + let target = args.target.clone(); + let issue = progress::run(&mut cx.err, "Fetching issue", move || { + gh.view_issue(&dir, &target) + })?; + + // An issue already linked to a branch reuses it, which costs no LLM call at + // all — the common case of returning to work in progress. + let linked = linked_branch(session.repo.gix(), issue.number)?; + let mut plan = match &linked { + Some(branch) => { + if let Some(requested) = &args.branch + && requested != branch + { + return Err(Error::usage(format!( + "issue #{} is already linked to branch {branch:?}; \ + drop --branch or pass that branch", + issue.number + ))); + } + cx.err.line(&format!( + "issue #{} is already linked to branch {branch:?}", + issue.number + ))?; + IssuePlan { + branch: branch.clone(), + brief: args.brief.clone().unwrap_or_default(), + } + } + None => resolve_plan(cx, &session, &issue, args)?, + }; + + let mut base = args + .from + .clone() + .or_else(|| default_base_ref(session.repo.gix())); + + // Review the proposal. Fields the user pinned with a flag are not re-asked. + if !cx.assume_yes { + let branches = all_branches(session.repo.gix()).unwrap_or_default(); + if args.branch.is_none() && linked.is_none() { + plan.branch = prompt_value(cx, "branch", &plan.branch)?; + } + if args.from.is_none() { + base = prompt_base(cx, base.as_deref(), &branches)?; + } + if args.brief.is_none() { + plan.brief = prompt_value(cx, "brief", &plan.brief)?; + } + } + + // A human-chosen name only has to be a legal git branch. The naming contract + // exists to constrain the *model*; the issue link lives in git config, never + // in the branch name, so an off-contract name still resolves back to its + // issue and breaks nothing. + validate_branch_name(&plan.branch).map_err(Error::usage)?; + if naming::parse_and_validate(&plan.branch, issue.number).is_err() { + cx.err.line(&format!( + "note: {:?} does not follow TYPE/{}-SLUG; the issue link is recorded \ + in git config, so this still works", + plan.branch, issue.number + ))?; + } + + let env = cx.env.clone(); + let preview = preview_target(&session.parts(&env), &plan.branch)?; + let prompt = format!( + "Issue #{}: {}\nBranch: {}\nBase: {}\nWorktree: {}\nBrief: {}\n\ + Create this issue worktree? [y/N] ", + issue.number, + issue.title, + plan.branch, + base.as_deref().unwrap_or("(repository default)"), + preview.display(), + if plan.brief.trim().is_empty() { + "(none)" + } else { + plan.brief.trim() + }, + ); + if !confirm(cx, &prompt)? { + cx.err.line("aborted: issue worktree was not created")?; + return Ok(0); + } + + let options = CreateOptions { + branch: plan.branch.clone(), + base, + track: None, + copy_from: args.copy_from.clone(), + // The service never prompts; the interactive submodule policy is applied + // below, exactly as `wt new` does it. + init_submodules: false, + seed_submodules: session.config.submodules_seed.is_enabled(), + reflink: session.config.create_reflink.is_enabled(), + no_hooks: args.no_hooks, + }; + let created = create_in(&session.parts(&env), git.as_ref(), hooks, &options)?; + report_created(cx, &created)?; + if !created.reused { + super::maybe_init_submodules_interactive( + cx, + git.as_ref(), + &created.path, + session.config.submodules_init, + args.submodule_override(), + !cx.assume_yes, + session.config.submodules_seed.is_enabled(), + )?; + } + + link_issue( + git.as_ref(), + &session.primary_root, + &created.branch, + &issue, + &plan.brief, + )?; + + let ctx = HookContext { + worktree_path: created.path.clone(), + branch: created.branch.clone(), + repo_root: session.primary_root.clone(), + base_ref: created.base_ref.clone(), + pr_number: None, + }; + finish_worktree( + cx, + hooks, + &created.path, + &ctx, + Nav { + json: false, + no_switch: args.no_switch, + note: if created.reused { + "reusing issue worktree at" + } else { + "created issue worktree at" + }, + start: None, + }, + ) +} + +/// Proposes a branch and brief for `issue`, never failing for a model reason. +/// +/// Only a bad `--model`/`--effort` value returns `Err`; every generation failure +/// mode degrades instead: +/// +/// | outcome | branch | brief | +/// | --- | --- | --- | +/// | valid | as generated | as generated | +/// | malformed branch | [`naming::fallback`] | kept — still usable | +/// | unavailable / timeout / unparseable | [`naming::fallback`] | empty | +fn resolve_plan( + cx: &mut Cx, + session: &Session, + issue: &IssueView, + args: &IssueArgs, +) -> Result { + let fallback = naming::fallback(fallback_kind(issue), issue.number, &issue.title); + + // Nothing left to generate: both values were supplied. + if let (Some(branch), Some(brief)) = (&args.branch, &args.brief) { + cx.err + .line("generation: skipped (branch and brief supplied)")?; + return Ok(IssuePlan { + branch: branch.clone(), + brief: brief.clone(), + }); + } + + let opts = resolve_generation_options(args, &session.config)?; + let kind = session.config.agent_generation.provider; + let agent = cx.agent.clone(); + let root = session.primary_root.clone(); + let prompt = build_generation_prompt(issue); + let generated = progress::run(&mut cx.err, "Generating issue setup", move || { + generate(agent.as_ref(), kind, &prompt, &root, &opts) + }); + + let (branch, brief) = match generated { + Ok(plan) => match naming::parse_and_validate(&plan.branch, issue.number) { + Ok(name) => (name.to_string(), plan.brief), + // Only the branch was wrong; the brief is independent and still useful. + Err(e) => { + cx.err.line(&format!( + "note: generated branch {:?} rejected ({e}); using {fallback} instead", + plan.branch + ))?; + (fallback.to_string(), plan.brief) + } + }, + Err(e) => { + cx.err.line(&format!( + "note: could not generate issue setup ({e}); using {fallback} with no brief" + ))?; + (fallback.to_string(), String::new()) + } + }; + + Ok(IssuePlan { + branch: args.branch.clone().unwrap_or(branch), + brief: args.brief.clone().unwrap_or(brief), + }) +} + +/// The branch kind implied by the issue's labels, then its issue type, then +/// `feat` (issue #98). Deterministic, so the fallback branch for a given issue +/// is always the same name. +fn fallback_kind(issue: &IssueView) -> BranchKind { + issue + .labels + .iter() + .map(|label| label.name.as_str()) + .chain(issue.issue_type.iter().map(|ty| ty.name.as_str())) + .find_map(BranchKind::from_label) + .unwrap_or(BranchKind::Feat) +} + +/// Resolves the generation model and effort: a `--model`/`--effort` flag +/// overrides `[agent.generation]`. An unknown flag value is a usage error — the +/// one failure in this flow that is the user's to fix, so it is *not* swallowed +/// by the fallback. +fn resolve_generation_options(args: &IssueArgs, config: &Config) -> Result { + let model = match &args.model { + Some(m) => AgentModel::parse(m).ok_or_else(|| { + Error::usage(format!( + "unknown --model {m:?}; expected one of: opus, sonnet, haiku" + )) + })?, + None => config.agent_generation.model, + }; + let effort = match &args.effort { + Some(e) => Effort::parse(e).ok_or_else(|| { + Error::usage(format!( + "unknown --effort {e:?}; expected one of: low, medium, high" + )) + })?, + None => config.agent_generation.effort, + }; + Ok(AgentOptions { + model, + effort, + timeout: Some(GENERATION_TIMEOUT), + }) +} + +/// The generation prompt. +/// +/// The branch contract is [`naming::branch_rule`] verbatim, so the rule the +/// model is asked to follow is the same one [`naming::parse_and_validate`] +/// enforces. Only the issue fields `wt` actually fetched are included; comments, +/// assignees and project data are deliberately absent. +fn build_generation_prompt(issue: &IssueView) -> String { + let labels = issue + .labels + .iter() + .map(|label| label.name.as_str()) + .collect::>() + .join(", "); + let issue_type = issue.issue_type.as_ref().map_or("", |ty| ty.name.as_str()); + let milestone = issue.milestone.as_ref().map_or("", |m| m.title.as_str()); + format!( + "Generate setup values for a coding agent working on a GitHub issue.\n\ + Return only one JSON object with string fields `branch` and `brief`.\n\ + {}\n\ + Keep `brief` under 120 words and describe the implementation outcome and \ + validation, without inventing requirements.\n\n\ + Issue number: {}\nURL: {}\nTitle: {}\nType: {}\nLabels: {}\nMilestone: {}\nBody:\n{}", + naming::branch_rule(issue.number), + issue.number, + issue.url, + issue.title, + issue_type, + labels, + milestone, + issue.body, + ) +} + +/// Runs the generation agent and parses its answer. Every failure here is +/// recoverable by the caller's fallback. +fn generate( + agent: &dyn AgentClient, + kind: AgentKind, + prompt: &str, + dir: &std::path::Path, + opts: &AgentOptions, +) -> Result { + let run = agent.run(kind, prompt, dir, opts)?; + if run.is_error { + return Err(Error::operation(format!( + "generation agent reported an error: {}", + run.result.trim() + ))); + } + parse_issue_plan(&run.result) +} + +/// Parses the generated JSON object, tolerating a markdown code fence around it +/// (models add one often enough that rejecting it would waste a good answer). +fn parse_issue_plan(output: &str) -> Result { + let trimmed = output.trim(); + let body = trimmed + .strip_prefix("```json") + .or_else(|| trimmed.strip_prefix("```")) + .and_then(|rest| rest.trim_end().strip_suffix("```")) + .unwrap_or(trimmed) + .trim(); + + #[derive(serde::Deserialize)] + struct Raw { + branch: String, + #[serde(default)] + brief: String, + } + let raw: Raw = serde_json::from_str(body).map_err(|e| { + Error::operation(format!("generated output was not the expected JSON: {e}")) + })?; + Ok(IssuePlan { + branch: raw.branch.trim().to_string(), + brief: raw.brief.trim().to_string(), + }) +} + +/// The local branch already linked to `issue_number`, if any. More than one is +/// ambiguous and refused rather than guessed at. +fn linked_branch(repo: &gix::Repository, issue_number: u64) -> Result> { + let mut found: Vec = local_branches(repo) + .unwrap_or_default() + .into_iter() + .filter(|branch| wtconfig::read_meta(repo, branch).issue_number == Some(issue_number)) + .collect(); + match found.len() { + 0 => Ok(None), + 1 => Ok(found.pop()), + _ => { + found.sort(); + Err(Error::operation(format!( + "issue #{issue_number} is linked to more than one branch: {}", + found.join(", ") + ))) + } + } +} + +/// Records the issue link for `branch` as a locked read-check-write. +/// +/// Two traps this avoids. First, `create_in` wrote metadata through the `git` +/// subprocess but `gix` snapshots config at open, so the session's handle is +/// stale — the check has to run against a freshly discovered repository. +/// Second, this cannot go through `Workspace::write_meta`, which takes the lock +/// itself and so cannot enclose the preceding read; the advisory lock is neither +/// reentrant nor owner-aware, so taking it twice would deadlock until it times +/// out. +fn link_issue( + git: &dyn GitCli, + root: &std::path::Path, + branch: &str, + issue: &IssueView, + brief: &str, +) -> Result<()> { + let repo = Repo::discover(root)?; + wtconfig::ensure_schema_supported(repo.gix())?; + let _lock = lock_repo(root)?; + + let meta = wtconfig::read_meta(repo.gix(), branch); + if let Some(other) = meta.issue_number + && other != issue.number + { + return Err(Error::operation(format!( + "branch {branch:?} is already linked to issue #{other}" + ))); + } + + apply_meta( + git, + root, + branch, + &MetaUpdate { + issue_number: Some(issue.number), + issue_title: Some(issue.title.clone()), + issue_url: Some(issue.url.clone()), + // An empty brief leaves the key alone rather than recording a blank. + issue_brief: (!brief.trim().is_empty()).then(|| brief.trim().to_string()), + ..MetaUpdate::default() + }, + ) +} + +/// Prompts for one field on stderr, keeping `current` when the line is blank. +fn prompt_value(cx: &mut Cx, label: &str, current: &str) -> Result { + let shown = if current.trim().is_empty() { + "(none)".to_string() + } else { + current.trim().to_string() + }; + cx.err.text(&format!("{label} [{shown}]: "))?; + cx.err.flush()?; + let line = cx.input.read_line()?; + let typed = line.trim(); + Ok(if typed.is_empty() { + current.to_string() + } else { + typed.to_string() + }) +} + +/// Prompts for the base ref as a numbered menu of the repository's branches, +/// with the repository default and a free-form entry. +fn prompt_base(cx: &mut Cx, current: Option<&str>, branches: &[String]) -> Result> { + let shown = current.unwrap_or("(repository default)"); + cx.err.line(&format!("base [{shown}]:"))?; + for (index, branch) in branches.iter().enumerate() { + let marker = if Some(branch.as_str()) == current { + " (current)" + } else { + "" + }; + cx.err.line(&format!(" {}) {branch}{marker}", index + 1))?; + } + cx.err + .line(" or type a ref; blank keeps the current value")?; + cx.err.text("base: ")?; + cx.err.flush()?; + + let line = cx.input.read_line()?; + let typed = line.trim(); + if typed.is_empty() { + return Ok(current.map(str::to_string)); + } + // A bare number selects from the menu; anything else is a ref, so a branch + // literally named "2" is still reachable by typing it at a non-menu prompt. + if let Ok(choice) = typed.parse::() { + return match branches.get(choice.wrapping_sub(1)) { + Some(branch) => Ok(Some(branch.clone())), + None => Err(Error::usage(format!( + "{choice} is not one of the {} listed branches", + branches.len() + ))), + }; + } + Ok(Some(typed.to_string())) +} + +/// The worktree path for `branch`, for tests that assert what was created. +#[cfg(test)] +fn target_of(session: &Session, env: &crate::cx::Env, branch: &str) -> std::path::PathBuf { + preview_target(&session.parts(env), branch).unwrap_or_default() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::cx::Stream; + use crate::gh::{IssueLabel, IssueType}; + use crate::git::cli::RealGit; + use crate::hooks::RealHookRunner; + use crate::testutil::{CannedInput, FakeAgent, FakeGh, SharedBuf, TestRepo, test_cx}; + use std::sync::Arc; + + fn issue() -> IssueView { + IssueView { + number: 7, + title: "Add login page".into(), + body: "Users need to sign in.".into(), + state: "OPEN".into(), + labels: vec![IssueLabel { + name: "enhancement".into(), + }], + issue_type: None, + milestone: None, + created_at: String::new(), + updated_at: String::new(), + url: "https://github.com/o/r/issues/7".into(), + } + } + + fn args() -> IssueArgs { + IssueArgs { + target: "7".into(), + branch: None, + from: None, + brief: None, + model: None, + effort: None, + no_switch: true, + no_hooks: true, + copy_from: None, + init_submodules: false, + no_init_submodules: true, + } + } + + /// A repo plus a `--yes` context wired to the given agent output. + fn run_with_agent( + agent: FakeAgent, + args: &IssueArgs, + ) -> (TestRepo, crate::testutil::TestCx, u8) { + let repo = TestRepo::init(); + let mut t = test_cx(&[], repo.root().to_str().unwrap_or_default()); + t.cx.assume_yes = true; + t.cx.gh = Arc::new(FakeGh::with_issue(issue())); + t.cx.agent = Arc::new(agent); + let code = run(&mut t.cx, &RealHookRunner, args).unwrap(); + (repo, t, code) + } + + /// The `wt..*` metadata recorded for `branch`. + fn meta(repo: &TestRepo, branch: &str) -> crate::config::wtconfig::WtMeta { + let r = Repo::discover(repo.root()).unwrap(); + wtconfig::read_meta(r.gix(), branch) + } + + fn branches(repo: &TestRepo) -> Vec { + let r = Repo::discover(repo.root()).unwrap(); + local_branches(r.gix()).unwrap_or_default() + } + + #[test] + fn generation_creates_the_branch_and_links_the_issue() { + let (repo, t, code) = run_with_agent( + FakeAgent::drafting(r#"{"branch":"feat/7-add-login","brief":"Wire up the form."}"#), + &args(), + ); + assert_eq!(code, 0); + assert!(branches(&repo).iter().any(|b| b == "feat/7-add-login")); + let recorded = meta(&repo, "feat/7-add-login"); + assert_eq!(recorded.issue_number, Some(7)); + assert_eq!(recorded.issue_title.as_deref(), Some("Add login page")); + assert_eq!( + recorded.issue_url.as_deref(), + Some("https://github.com/o/r/issues/7") + ); + assert_eq!(recorded.issue_brief.as_deref(), Some("Wire up the form.")); + assert!(t.err.contents().contains("created issue worktree at")); + } + + #[test] + fn a_malformed_branch_falls_back_but_keeps_the_brief() { + // Only the branch violated the contract; the brief is independent. + let (repo, t, _) = run_with_agent( + FakeAgent::drafting(r#"{"branch":"NOT A BRANCH","brief":"Wire up the form."}"#), + &args(), + ); + assert!(branches(&repo).iter().any(|b| b == "feat/7-add-login-page")); + assert_eq!( + meta(&repo, "feat/7-add-login-page").issue_brief.as_deref(), + Some("Wire up the form.") + ); + assert!( + t.err.contents().contains("rejected"), + "{}", + t.err.contents() + ); + } + + #[test] + fn garbage_output_still_produces_a_valid_branch_and_worktree() { + // Issue #98's headline requirement: a flaky generation call must not be + // able to block worktree creation. + let (repo, t, code) = + run_with_agent(FakeAgent::drafting("I'm afraid I can't do that"), &args()); + assert_eq!(code, 0); + assert!(branches(&repo).iter().any(|b| b == "feat/7-add-login-page")); + assert_eq!(meta(&repo, "feat/7-add-login-page").issue_number, Some(7)); + // A failed generation leaves no brief rather than a bogus one. + assert_eq!(meta(&repo, "feat/7-add-login-page").issue_brief, None); + assert!(t.err.contents().contains("could not generate")); + } + + #[test] + fn an_unavailable_agent_falls_back() { + let (repo, _, code) = run_with_agent(FakeAgent::unavailable(), &args()); + assert_eq!(code, 0); + assert!(branches(&repo).iter().any(|b| b == "feat/7-add-login-page")); + } + + #[test] + fn a_timed_out_agent_falls_back() { + let (repo, t, code) = run_with_agent(FakeAgent::timing_out(), &args()); + assert_eq!(code, 0); + assert!(branches(&repo).iter().any(|b| b == "feat/7-add-login-page")); + assert!(t.err.contents().contains("did not respond")); + } + + #[test] + fn an_erroring_agent_falls_back() { + let (repo, _, code) = run_with_agent(FakeAgent::erroring("rate limited"), &args()); + assert_eq!(code, 0); + assert!(branches(&repo).iter().any(|b| b == "feat/7-add-login-page")); + } + + #[test] + fn explicit_branch_and_brief_skip_generation_entirely() { + let mut a = args(); + a.branch = Some("fix/7-mine".into()); + a.brief = Some("Do the thing.".into()); + // An unavailable agent proves no generation call was attempted. + let (repo, t, code) = run_with_agent(FakeAgent::unavailable(), &a); + assert_eq!(code, 0); + assert!(branches(&repo).iter().any(|b| b == "fix/7-mine")); + assert_eq!( + meta(&repo, "fix/7-mine").issue_brief.as_deref(), + Some("Do the thing.") + ); + assert!(t.err.contents().contains("generation: skipped")); + } + + #[test] + fn fallback_kind_prefers_labels_then_type_then_feat() { + let mut i = issue(); + i.labels = vec![IssueLabel { name: "bug".into() }]; + assert_eq!(fallback_kind(&i), BranchKind::Fix); + + // Case-insensitive, and unknown labels fall through to the next source. + i.labels = vec![IssueLabel { + name: "Documentation".into(), + }]; + assert_eq!(fallback_kind(&i), BranchKind::Docs); + + i.labels = vec![IssueLabel { + name: "help wanted".into(), + }]; + i.issue_type = Some(IssueType { + name: "Feature".into(), + }); + assert_eq!(fallback_kind(&i), BranchKind::Feat); + + i.labels = Vec::new(); + i.issue_type = None; + assert_eq!(fallback_kind(&i), BranchKind::Feat); + } + + #[test] + fn an_already_linked_branch_is_reused_without_generation() { + let repo = TestRepo::init(); + repo.git(&["branch", "feat/7-existing"]); + wtconfig::write_issue_number(&RealGit, repo.root(), "feat/7-existing", 7).unwrap(); + + let mut t = test_cx(&[], repo.root().to_str().unwrap_or_default()); + t.cx.assume_yes = true; + t.cx.gh = Arc::new(FakeGh::with_issue(issue())); + // Unavailable: reaching generation at all would fail this expectation. + t.cx.agent = Arc::new(FakeAgent::unavailable()); + let code = run(&mut t.cx, &RealHookRunner, &args()).unwrap(); + + assert_eq!(code, 0); + assert!(t.err.contents().contains("already linked to branch")); + assert!(!t.err.contents().contains("could not generate")); + } + + #[test] + fn a_branch_flag_conflicting_with_the_link_is_a_usage_error() { + let repo = TestRepo::init(); + repo.git(&["branch", "feat/7-existing"]); + wtconfig::write_issue_number(&RealGit, repo.root(), "feat/7-existing", 7).unwrap(); + + let mut t = test_cx(&[], repo.root().to_str().unwrap_or_default()); + t.cx.assume_yes = true; + t.cx.gh = Arc::new(FakeGh::with_issue(issue())); + let mut a = args(); + a.branch = Some("feat/7-other".into()); + let err = run(&mut t.cx, &RealHookRunner, &a).unwrap_err(); + assert_eq!(err.exit_code(), 2); + assert!(err.to_string().contains("already linked")); + } + + #[test] + fn a_branch_linked_to_another_issue_is_refused() { + let repo = TestRepo::init(); + let mut t = test_cx(&[], repo.root().to_str().unwrap_or_default()); + t.cx.assume_yes = true; + t.cx.gh = Arc::new(FakeGh::with_issue(issue())); + t.cx.agent = Arc::new(FakeAgent::unavailable()); + + let mut a = args(); + a.branch = Some("feat/shared".into()); + a.brief = Some("x".into()); + repo.git(&["branch", "feat/shared"]); + wtconfig::write_issue_number(&RealGit, repo.root(), "feat/shared", 9).unwrap(); + + let err = run(&mut t.cx, &RealHookRunner, &a).unwrap_err(); + assert!( + err.to_string().contains("already linked to issue #9"), + "{err}" + ); + } + + #[test] + fn without_a_terminal_or_yes_it_fails_before_the_fetch() { + let repo = TestRepo::init(); + let mut t = test_cx(&[], repo.root().to_str().unwrap_or_default()); + // An unavailable `gh` would make a fetch fail loudly; a `Usage` error + // instead proves the gate ran first. + t.cx.gh = Arc::new(FakeGh::unavailable()); + let err = run(&mut t.cx, &RealHookRunner, &args()).unwrap_err(); + assert_eq!(err.exit_code(), 2); + assert!(err.to_string().contains("interactive terminal")); + } + + /// A `--yes`-free context whose stderr is a TTY, so the review prompts run. + fn interactive(repo: &TestRepo, answers: &[&str]) -> crate::testutil::TestCx { + let mut t = test_cx(&[], repo.root().to_str().unwrap_or_default()); + let err = SharedBuf::new(); + t.cx.err = Stream::new(Box::new(err.clone()), true); + t.err = err; + t.cx.input = Box::new(CannedInput::new(answers)); + t.cx.gh = Arc::new(FakeGh::with_issue(issue())); + t + } + + #[test] + fn declining_the_confirmation_creates_nothing() { + let repo = TestRepo::init(); + // branch, base, brief, then decline. + let mut t = interactive(&repo, &["", "", "", "n"]); + t.cx.agent = Arc::new(FakeAgent::drafting( + r#"{"branch":"feat/7-add-login","brief":"b"}"#, + )); + let code = run(&mut t.cx, &RealHookRunner, &args()).unwrap(); + + assert_eq!(code, 0); + assert!(t.err.contents().contains("aborted")); + assert!(!branches(&repo).iter().any(|b| b == "feat/7-add-login")); + assert_eq!(meta(&repo, "feat/7-add-login").issue_number, None); + } + + #[test] + fn interactive_edits_override_the_generated_values() { + let repo = TestRepo::init(); + let mut t = interactive(&repo, &["fix/7-mine", "", "my brief", "y"]); + t.cx.agent = Arc::new(FakeAgent::drafting( + r#"{"branch":"feat/7-add-login","brief":"generated"}"#, + )); + let code = run(&mut t.cx, &RealHookRunner, &args()).unwrap(); + + assert_eq!(code, 0); + assert!(branches(&repo).iter().any(|b| b == "fix/7-mine")); + assert_eq!( + meta(&repo, "fix/7-mine").issue_brief.as_deref(), + Some("my brief") + ); + } + + #[test] + fn an_edited_off_contract_branch_warns_but_proceeds() { + let repo = TestRepo::init(); + let mut t = interactive(&repo, &["hotfix-thing", "", "", "y"]); + t.cx.agent = Arc::new(FakeAgent::drafting( + r#"{"branch":"feat/7-add-login","brief":"b"}"#, + )); + let code = run(&mut t.cx, &RealHookRunner, &args()).unwrap(); + + assert_eq!(code, 0); + assert!(branches(&repo).iter().any(|b| b == "hotfix-thing")); + assert!(t.err.contents().contains("does not follow TYPE/7-SLUG")); + // The link still resolves, which is why the warning is not an error. + assert_eq!(meta(&repo, "hotfix-thing").issue_number, Some(7)); + } + + #[test] + fn an_edited_illegal_branch_name_is_still_refused() { + let repo = TestRepo::init(); + let mut t = interactive(&repo, &["not a branch", "", "", "y"]); + t.cx.agent = Arc::new(FakeAgent::drafting( + r#"{"branch":"feat/7-add-login","brief":"b"}"#, + )); + let err = run(&mut t.cx, &RealHookRunner, &args()).unwrap_err(); + assert_eq!(err.exit_code(), 2); + } + + #[test] + fn the_prompt_carries_the_naming_contract_verbatim() { + let prompt = build_generation_prompt(&issue()); + assert!(prompt.contains(&naming::branch_rule(7))); + assert!(prompt.contains("Add login page")); + assert!(prompt.contains("Users need to sign in.")); + assert!(prompt.contains("enhancement")); + // Only the fetched fields; nothing invented. + assert!(!prompt.contains("comments")); + } + + #[test] + fn plan_parsing_tolerates_code_fences() { + let bare = parse_issue_plan(r#"{"branch":"feat/7-x","brief":"b"}"#).unwrap(); + assert_eq!(bare.branch, "feat/7-x"); + let fenced = + parse_issue_plan("```json\n{\"branch\":\"feat/7-x\",\"brief\":\"b\"}\n```").unwrap(); + assert_eq!(fenced, bare); + let plain = + parse_issue_plan("```\n{\"branch\":\"feat/7-x\",\"brief\":\"b\"}\n```").unwrap(); + assert_eq!(plain, bare); + // A missing brief is tolerated; a missing branch is not. + assert_eq!( + parse_issue_plan(r#"{"branch":"feat/7-x"}"#).unwrap().brief, + "" + ); + assert!(parse_issue_plan(r#"{"brief":"b"}"#).is_err()); + assert!(parse_issue_plan("not json at all").is_err()); + } + + #[test] + fn generation_flags_override_the_config_profile() { + let config = Config::default(); + let mut a = args(); + let opts = resolve_generation_options(&a, &config).unwrap(); + assert_eq!(opts.model, config.agent_generation.model); + assert_eq!(opts.effort, config.agent_generation.effort); + // Generation is always bounded, so a hung agent cannot block a worktree. + assert_eq!(opts.timeout, Some(GENERATION_TIMEOUT)); + + a.model = Some("opus".into()); + a.effort = Some("high".into()); + let opts = resolve_generation_options(&a, &config).unwrap(); + assert_eq!(opts.model, AgentModel::Opus); + assert_eq!(opts.effort, Effort::High); + + a.model = Some("gpt".into()); + let err = resolve_generation_options(&a, &config).unwrap_err(); + assert_eq!(err.exit_code(), 2); + } + + #[test] + fn a_bad_model_flag_fails_rather_than_falling_back() { + // The one failure in this flow that is the user's to fix. + let repo = TestRepo::init(); + let mut t = test_cx(&[], repo.root().to_str().unwrap_or_default()); + t.cx.assume_yes = true; + t.cx.gh = Arc::new(FakeGh::with_issue(issue())); + let mut a = args(); + a.effort = Some("nonsense".into()); + let err = run(&mut t.cx, &RealHookRunner, &a).unwrap_err(); + assert_eq!(err.exit_code(), 2); + } + + #[test] + fn base_choice_accepts_a_number_a_ref_and_a_blank() { + let repo = TestRepo::init(); + let all = vec!["main".to_string(), "topic".to_string()]; + + let mut t = interactive(&repo, &["2"]); + assert_eq!( + prompt_base(&mut t.cx, Some("main"), &all).unwrap(), + Some("topic".to_string()) + ); + + let mut t = interactive(&repo, &["origin/release"]); + assert_eq!( + prompt_base(&mut t.cx, Some("main"), &all).unwrap(), + Some("origin/release".to_string()) + ); + + let mut t = interactive(&repo, &[""]); + assert_eq!( + prompt_base(&mut t.cx, Some("main"), &all).unwrap(), + Some("main".to_string()) + ); + + let mut t = interactive(&repo, &["9"]); + let err = prompt_base(&mut t.cx, Some("main"), &all).unwrap_err(); + assert_eq!(err.exit_code(), 2); + } + + #[test] + fn a_field_prompt_keeps_the_current_value_on_a_blank_line() { + let repo = TestRepo::init(); + let mut t = interactive(&repo, &["", "typed"]); + assert_eq!(prompt_value(&mut t.cx, "branch", "kept").unwrap(), "kept"); + assert_eq!(prompt_value(&mut t.cx, "branch", "kept").unwrap(), "typed"); + assert!(t.err.contents().contains("branch [kept]: ")); + } + + #[test] + fn the_worktree_lands_where_the_confirmation_previewed() { + let (repo, t, _) = run_with_agent( + FakeAgent::drafting(r#"{"branch":"feat/7-add-login","brief":"b"}"#), + &args(), + ); + let mut probe = test_cx(&[], repo.root().to_str().unwrap_or_default()); + probe.cx.assume_yes = true; + let session = open_session(&probe.cx, &RealGit).unwrap(); + let expected = target_of(&session, &probe.cx.env.clone(), "feat/7-add-login"); + assert!( + expected.is_dir(), + "{} is not a directory", + expected.display() + ); + assert!(t.err.contents().contains(&expected.display().to_string())); + } +} diff --git a/src/commands/mod.rs b/src/commands/mod.rs index ae78889..d948181 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -7,6 +7,7 @@ pub mod completions; pub mod config_cmd; pub mod drop; pub mod init; +pub mod issue; pub mod list; pub mod new; pub mod path; @@ -32,7 +33,7 @@ use crate::git::discover::Repo; use crate::hooks::{HookContext, HookRunner}; use crate::model::Worktree; use crate::query::{self, Resolved}; -use crate::worktree::{Workspace, WorkspaceParts, build_worktrees}; +use crate::worktree::{CreatedWorktree, HookOutcome, Workspace, WorkspaceParts, build_worktrees}; // Path/template helpers shared with the service layer, re-exported so command // modules keep their historical import paths. @@ -396,6 +397,29 @@ pub(crate) fn finish_worktree( crate::hooks::run_start(runner, command, hook_ctx) } +/// Reports the non-fatal outcomes of a completed creation: the copy step and +/// the `post_create` hook. +/// +/// The hook already ran inside the service, where a failure is a warning rather +/// than a rollback (spec §8). Shared by `wt new` and `wt issue` so both report +/// the same way. +pub(crate) fn report_created(cx: &mut Cx, created: &CreatedWorktree) -> Result<()> { + log_copy_outcome(cx, &created.copy); + match &created.post_create { + HookOutcome::ExitedNonZero(code) => { + cx.err.line(&format!( + "warning: post_create hook exited with status {code}" + ))?; + } + HookOutcome::Failed(error) => { + cx.err + .line(&format!("warning: post_create hook failed: {error}"))?; + } + HookOutcome::Skipped | HookOutcome::Succeeded => {} + } + Ok(()) +} + /// Writes `target` to the file named by `$WT_CD_FILE` so the shell wrapper can /// `cd` there once `wt` exits (issue #89). /// diff --git a/src/commands/new.rs b/src/commands/new.rs index 579b4bd..aebd90d 100644 --- a/src/commands/new.rs +++ b/src/commands/new.rs @@ -7,7 +7,7 @@ use crate::error::Result; use crate::git::discover::Repo; use crate::git::{branch_ref, resolve_hex}; use crate::hooks::{HookContext, HookRunner}; -use crate::worktree::{CreateOptions, HookOutcome, create_in, resolve_base}; +use crate::worktree::{CreateOptions, create_in, resolve_base}; /// Creates a linked worktree for `branch`, prompting first when the base it would /// fork from is behind its origin counterpart (issue #56): the user can update the @@ -95,21 +95,7 @@ pub(crate) fn run_core( let env = cx.env.clone(); let created = create_in(&session.parts(&env), git, hooks, &options)?; - crate::commands::log_copy_outcome(cx, &created.copy); - // The post-create hook already ran in the service; a failure is a warning, - // not a rollback (§8). - match &created.post_create { - HookOutcome::ExitedNonZero(code) => { - cx.err.line(&format!( - "warning: post_create hook exited with status {code}" - ))?; - } - HookOutcome::Failed(error) => { - cx.err - .line(&format!("warning: post_create hook failed: {error}"))?; - } - HookOutcome::Skipped | HookOutcome::Succeeded => {} - } + crate::commands::report_created(cx, &created)?; // Initialize submodules per the policy/flag, prompting (default yes) at an // interactive terminal when the policy is left at its default (issue #50). diff --git a/src/git/mod.rs b/src/git/mod.rs index d6597ea..ab24817 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -27,10 +27,12 @@ pub(crate) use refs::{ validate_branch_name, }; // Only the command handlers reach for these; the core library does not. +// `default_base_ref` moved here from the TUI group when `wt issue` (a CLI +// command) started resolving its base from `origin/HEAD`. #[cfg(feature = "cli")] -pub(crate) use refs::{all_branches, current_branch, remote_branches}; -// Only the TUI reaches for these. +pub(crate) use refs::{all_branches, current_branch, default_base_ref, remote_branches}; +// Only the TUI reaches for this. #[cfg(feature = "tui")] -pub(crate) use refs::{default_base_ref, origin_head_branch}; +pub(crate) use refs::origin_head_branch; pub(crate) use status::status_of; pub(crate) use worktrees::enumerate; diff --git a/src/lib.rs b/src/lib.rs index 2313876..127a26e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,6 +26,9 @@ pub mod keys; pub mod model; pub mod naming; pub mod output; +/// Human-facing progress for slow foreground operations (CLI only). +#[cfg(feature = "cli")] +pub(crate) mod progress; pub mod query; pub mod slug; pub mod template; diff --git a/src/progress.rs b/src/progress.rs new file mode 100644 index 0000000..c6bed24 --- /dev/null +++ b/src/progress.rs @@ -0,0 +1,96 @@ +//! Human-facing progress for slow foreground operations. + +use std::sync::mpsc; +use std::time::{Duration, Instant}; + +use crate::cx::Stream; +use crate::error::{Error, Result}; + +const FRAMES: &[&str] = &["|", "/", "-", "\\"]; +const TICK: Duration = Duration::from_millis(80); + +/// Runs `operation` while displaying an animated TTY spinner on stderr. +/// Non-TTY streams receive stable start/finish lines with no control codes. +pub(crate) fn run(stream: &mut Stream, label: &str, operation: F) -> Result +where + T: Send, + F: FnOnce() -> Result + Send, +{ + let started = Instant::now(); + if !stream.is_tty() { + stream.line(&format!("… {label}"))?; + let result = operation(); + finish_line(stream, label, started, result.is_ok())?; + return result; + } + + std::thread::scope(|scope| { + let (tx, rx) = mpsc::sync_channel(1); + scope.spawn(move || { + let _ = tx.send(operation()); + }); + let mut frame = 0; + let result = loop { + stream.text(&format!( + "\r\x1b[2K{} {label}", + FRAMES[frame % FRAMES.len()] + ))?; + stream.flush()?; + match rx.recv_timeout(TICK) { + Ok(result) => break result, + Err(mpsc::RecvTimeoutError::Timeout) => frame = frame.wrapping_add(1), + Err(mpsc::RecvTimeoutError::Disconnected) => { + break Err(Error::operation(format!("{label} task panicked"))); + } + } + }; + stream.text("\r\x1b[2K")?; + finish_line(stream, label, started, result.is_ok())?; + result + }) +} + +fn finish_line(stream: &mut Stream, label: &str, started: Instant, success: bool) -> Result<()> { + let marker = if success { "✓" } else { "✗" }; + stream.line(&format!( + "{marker} {label} ({:.1}s)", + started.elapsed().as_secs_f64() + )) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::testutil::SharedBuf; + + #[test] + fn non_tty_progress_is_stable_and_reports_success() { + let buffer = SharedBuf::new(); + let mut stream = Stream::new(Box::new(buffer.clone()), false); + assert_eq!(run(&mut stream, "Generating", || Ok(42)).unwrap(), 42); + let output = buffer.contents(); + assert!(output.starts_with("… Generating\n")); + assert!(output.contains("✓ Generating (")); + assert!(!output.contains("\x1b")); + } + + #[test] + fn non_tty_progress_reports_failure_and_preserves_error() { + let buffer = SharedBuf::new(); + let mut stream = Stream::new(Box::new(buffer.clone()), false); + let error = + run::<(), _>(&mut stream, "Generating", || Err(Error::operation("boom"))).unwrap_err(); + assert_eq!(error.to_string(), "boom"); + assert!(buffer.contents().contains("✗ Generating (")); + } + + #[test] + fn tty_progress_uses_spinner_and_clears_the_line() { + let buffer = SharedBuf::new(); + let mut stream = Stream::new(Box::new(buffer.clone()), true); + run(&mut stream, "Fetching", || Ok(())).unwrap(); + let output = buffer.contents(); + assert!(output.contains("\r\x1b[2K| Fetching")); + assert!(output.contains("✓ Fetching (")); + } +} diff --git a/src/worktree/mod.rs b/src/worktree/mod.rs index 2818868..1fe80a6 100644 --- a/src/worktree/mod.rs +++ b/src/worktree/mod.rs @@ -35,6 +35,6 @@ pub use service::{ }; #[cfg(feature = "cli")] pub(crate) use service::{ - WorkspaceParts, apply_meta, create_in, lock_repo, remove_in, resolve_base, resolve_target, - rollback_worktree, run_best_effort, same_path, + WorkspaceParts, apply_meta, create_in, lock_repo, preview_target, remove_in, resolve_base, + resolve_target, rollback_worktree, run_best_effort, same_path, }; diff --git a/src/worktree/service.rs b/src/worktree/service.rs index 483079f..54fdc5e 100644 --- a/src/worktree/service.rs +++ b/src/worktree/service.rs @@ -505,6 +505,20 @@ pub(crate) fn resolve_base(repo: &Repo, config: &Config, explicit: Option<&str>) ("HEAD".to_string(), true) } +/// The path `create_in` would materialize `branch` at, without creating +/// anything. +/// +/// Exists so a caller that previews the target before confirming (`wt issue`) +/// derives it from the same slug and template logic `create_in` uses, rather +/// than reimplementing it and drifting. +#[cfg(feature = "cli")] +pub(crate) fn preview_target(ws: &WorkspaceParts<'_>, branch: &str) -> Result { + let base_commit = resolve_hex(ws.repo.gix(), &branch_ref(branch)).unwrap_or_default(); + let short_hash = base_commit.get(..7).unwrap_or(&base_commit).to_string(); + let slug = slugify_with_fallback(branch, &short_hash); + render_target(ws.config, ws.root, branch, &slug, ws.env) +} + /// Creates (or reuses) a worktree per `options`; see [`Workspace::create`]. pub(crate) fn create_in( ws: &WorkspaceParts<'_>, From 3f3bb4431ef13314308063128b42a0c8f0ee2210 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Fri, 28 Aug 2026 20:36:39 -0400 Subject: [PATCH 6/9] feat(cli): complete issue numbers and route wt issue in the shell wrappers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the `issue-numbers` completion kind and wires `issue` into all five shell wrappers, so `wt issue ` offers open issue numbers and the resulting worktree is cd'd into like `wt new`'s. Completion is best-effort, exactly like `pr-numbers`: it runs on every keystroke, so an unauthenticated or unreachable `gh` yields an empty list rather than an error. A test pins that. Notably `issue` gets **no** inherited-stdio special case. It is an interactive command, which invites the assumption that it needs the `--start`-style `WT_CD_FILE` handoff, but the wrappers capture *stdout* only — stdin and stderr already reach the review prompts, exactly as `wt new`'s base-staleness and submodule prompts already rely on. Forcing the handoff would also make stdout a terminal, so the path would be printed to the user *and* cd'd into: double output. A regression test asserts no wrapper reintroduces it. Claude-Session: https://claude.ai/code/session_014ce9P64RKbVU6kj3GmA1mo --- src/commands/complete.rs | 44 ++++++++++++++++++ src/commands/shell_init.rs | 94 ++++++++++++++++++++++++++++++++++---- 2 files changed, 129 insertions(+), 9 deletions(-) diff --git a/src/commands/complete.rs b/src/commands/complete.rs index 0fc35dc..87831ad 100644 --- a/src/commands/complete.rs +++ b/src/commands/complete.rs @@ -63,6 +63,18 @@ fn candidates(cx: &Cx, kind: &str) -> Result> { .map(|n| n.to_string()) .collect()) } + "issue-numbers" => { + // Best-effort, exactly like `pr-numbers`: GitHub being unreachable + // must never make shell completion fail. + let dir = repo.current_workdir().unwrap_or_else(|| cx.cwd.clone()); + Ok(cx + .gh + .list_open_issues(&dir) + .unwrap_or_default() + .into_iter() + .map(|issue| issue.number.to_string()) + .collect()) + } _ => Ok(Vec::new()), } } @@ -70,6 +82,7 @@ fn candidates(cx: &Cx, kind: &str) -> Result> { #[cfg(test)] mod tests { use crate::cli::CompleteArgs; + use crate::gh::IssueSummary; use crate::testutil::TestRepo; fn args(kind: &str, partial: Option<&str>) -> CompleteArgs { @@ -102,6 +115,37 @@ mod tests { assert!(out.contains("topic")); } + #[test] + fn completes_open_issue_numbers() { + let repo = TestRepo::init(); + let mut t = crate::testutil::test_cx(&[], repo.root().to_str().unwrap()); + t.cx.gh = std::sync::Arc::new(crate::testutil::FakeGh::default().with_issue_list(vec![ + IssueSummary { + number: 7, + title: "Add login".into(), + state: "OPEN".into(), + labels: Vec::new(), + issue_type: None, + milestone: None, + created_at: String::new(), + url: String::new(), + }, + ])); + super::run(&mut t.cx, &args("issue-numbers", None)).unwrap(); + assert!(t.out.contents().contains('7')); + } + + #[test] + fn issue_completion_is_silent_when_github_is_unreachable() { + // Completion runs on every keystroke; an unauthenticated `gh` must + // produce an empty list, never an error. + let repo = TestRepo::init(); + let mut t = crate::testutil::test_cx(&[], repo.root().to_str().unwrap()); + t.cx.gh = std::sync::Arc::new(crate::testutil::FakeGh::unavailable()); + super::run(&mut t.cx, &args("issue-numbers", None)).unwrap(); + assert!(t.out.contents().trim().is_empty()); + } + #[test] fn completes_all_branches_including_remote() { // `checkout` completion (the `all-branches` kind) lists local branches and diff --git a/src/commands/shell_init.rs b/src/commands/shell_init.rs index 0384a00..859c5ee 100644 --- a/src/commands/shell_init.rs +++ b/src/commands/shell_init.rs @@ -45,7 +45,7 @@ pub(crate) fn run(cx: &mut Cx, args: &ShellInitArgs) -> Result { const BASH: &str = r#"# wt shell integration (bash) — source this from your ~/.bashrc wt() { case "${1-}" in - switch|sw|checkout|co|new|pr|drop|ui|tui|"") + switch|sw|checkout|co|new|issue|pr|drop|ui|tui|"") local __wt_arg for __wt_arg in "$@"; do case "$__wt_arg" in @@ -99,9 +99,16 @@ _wt_complete() { if [ "$COMP_CWORD" -eq 2 ]; then COMPREPLY=($(command wt __complete pr-numbers "$cur" 2>/dev/null)); return fi ;; + issue) + if [ "$prev" = "--from" ]; then + COMPREPLY=($(command wt __complete branches "$cur" 2>/dev/null)); return + fi + if [ "$COMP_CWORD" -eq 2 ]; then + COMPREPLY=($(command wt __complete issue-numbers "$cur" 2>/dev/null)); return + fi ;; esac if [ "$COMP_CWORD" -eq 1 ]; then - COMPREPLY=($(compgen -W "new checkout co list ls switch sw remove rm drop prune pr status path root init config completions shell-init ui tui" -- "$cur")) + COMPREPLY=($(compgen -W "new checkout co issue list ls switch sw remove rm drop prune pr status path root init config completions shell-init ui tui" -- "$cur")) fi } complete -F _wt_complete wt @@ -110,7 +117,7 @@ complete -F _wt_complete wt const ZSH: &str = r#"# wt shell integration (zsh) — source this from your ~/.zshrc wt() { case "${1-}" in - switch|sw|checkout|co|new|pr|drop|ui|tui|"") + switch|sw|checkout|co|new|issue|pr|drop|ui|tui|"") local __wt_arg for __wt_arg in "$@"; do case "$__wt_arg" in @@ -153,8 +160,10 @@ _wt() { compadd -- ${(f)"$(command wt __complete all-branches "${words[CURRENT]}" 2>/dev/null)"}; return ;; pr) compadd -- ${(f)"$(command wt __complete pr-numbers "${words[CURRENT]}" 2>/dev/null)"}; return ;; + issue) + compadd -- ${(f)"$(command wt __complete issue-numbers "${words[CURRENT]}" 2>/dev/null)"}; return ;; esac - compadd -- new checkout co list ls switch sw remove rm drop prune pr status path root init config completions shell-init ui tui + compadd -- new checkout co issue list ls switch sw remove rm drop prune pr status path root init config completions shell-init ui tui } compdef _wt wt "#; @@ -162,7 +171,7 @@ compdef _wt wt const FISH: &str = r#"# wt shell integration (fish) — source this from your config.fish function wt set -l cmd $argv[1] - if test (count $argv) -eq 0; or contains -- "$cmd" switch sw checkout co new pr drop ui tui + if test (count $argv) -eq 0; or contains -- "$cmd" switch sw checkout co new issue pr drop ui tui # --start runs a command in the new worktree, so it needs the real # terminal: run wt with stdio inherited and take the path via a file. if contains -- --start $argv; or string match -q -- '--start=*' $argv @@ -206,13 +215,15 @@ complete -c wt -n '__fish_seen_subcommand_from checkout co' \ -a '(command wt __complete all-branches 2>/dev/null)' complete -c wt -n '__fish_seen_subcommand_from pr' \ -a '(command wt __complete pr-numbers 2>/dev/null)' +complete -c wt -n '__fish_seen_subcommand_from issue' \ + -a '(command wt __complete issue-numbers 2>/dev/null)' complete -c wt -n '__fish_use_subcommand' \ - -a 'new checkout co list switch remove drop prune pr status path root init config completions shell-init ui tui' + -a 'new checkout co issue list switch remove drop prune pr status path root init config completions shell-init ui tui' "#; const POWERSHELL: &str = r#"# wt shell integration (PowerShell) — add to your $PROFILE function wt { - $nav = @('switch','sw','checkout','co','new','pr','drop','ui','tui') + $nav = @('switch','sw','checkout','co','new','issue','pr','drop','ui','tui') $exe = (Get-Command wt -CommandType Application | Select-Object -First 1).Source if ($args.Count -eq 0 -or $nav -contains $args[0]) { # --start runs a command in the new worktree, so it needs the real @@ -251,14 +262,15 @@ Register-ArgumentCompleter -CommandName wt -Native -ScriptBlock { '^new$' { & $exe __complete branches $wordToComplete } '^(checkout|co)$' { & $exe __complete all-branches $wordToComplete } '^pr$' { & $exe __complete pr-numbers $wordToComplete } - default { 'new','checkout','co','list','switch','remove','drop','prune','pr','status','path','root','init','config','completions','shell-init','ui','tui' | Where-Object { $_ -like "$wordToComplete*" } } + '^issue$' { & $exe __complete issue-numbers $wordToComplete } + default { 'new','checkout','co','issue','list','switch','remove','drop','prune','pr','status','path','root','init','config','completions','shell-init','ui','tui' | Where-Object { $_ -like "$wordToComplete*" } } } } "#; const ELVISH: &str = r#"# wt shell integration (elvish) — source this from your rc.elv fn wt {|@a| - var nav = [switch sw checkout co new pr drop ui tui] + var nav = [switch sw checkout co new issue pr drop ui tui] if (or (== (count $a) 0) (and (> (count $a) 0) (has-value $nav $a[0]))) { # --start runs a command in the new worktree, so it needs the real # terminal: run wt with stdio inherited and take the path via a file. @@ -480,6 +492,70 @@ mod tests { } } + #[test] + fn every_wrapper_routes_issue_like_new() { + // `wt issue` prints the worktree path on stdout like `wt new`, so it is + // cd-eligible in every wrapper and offered as a subcommand. + for shell in [ + Shell::Bash, + Shell::Zsh, + Shell::Fish, + Shell::PowerShell, + Shell::Elvish, + ] { + let s = snippet(shell); + assert!( + s.contains("issue"), + "{shell:?} wrapper does not wire up issue" + ); + } + } + + #[test] + fn no_wrapper_special_cases_issue_for_inherited_stdio() { + // Regression guard. `wt issue` is interactive, which invites the + // assumption that it needs the `--start`-style `WT_CD_FILE` handoff. It + // does not: the wrappers capture *stdout* only, so stdin and stderr + // already reach the review prompts — exactly as `wt new`'s own prompts + // rely on. Forcing the handoff would also make stdout a TTY, so the path + // would be printed *and* cd'd into: double output. + // + // `WT_CD_FILE` must therefore appear only under the `--start` branch. + for shell in [ + Shell::Bash, + Shell::Zsh, + Shell::Fish, + Shell::PowerShell, + Shell::Elvish, + ] { + let s = snippet(shell); + for line in s.lines() { + if line.contains("WT_CD_FILE") { + continue; + } + assert!( + !(line.contains("issue") && line.contains("mktemp")), + "{shell:?} appears to special-case issue for stdio: {line}" + ); + } + let handoffs = s.matches("WT_CD_FILE").count(); + assert!(handoffs > 0, "{shell:?} lost the --start handoff entirely"); + } + } + + #[test] + fn issue_completes_open_issue_numbers() { + // The shells with a subcommand-dispatch completion offer issue numbers + // the same way they offer PR numbers. + for shell in [Shell::Bash, Shell::Zsh, Shell::Fish, Shell::PowerShell] { + let s = snippet(shell); + assert!( + s.contains("__complete issue-numbers"), + "{shell:?} does not complete issue numbers" + ); + } + } + #[test] fn checkout_completes_local_and_remote_branches() { // `wt checkout` switches to a local *or* remote-only branch (DWIM), so its From 971112aa3399de0fa79424cc5c1e5b746c3fa7b8 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Fri, 28 Aug 2026 20:38:12 -0400 Subject: [PATCH 7/9] docs: document wt issue and the agent.generation profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the `wt issue` feature bullet and quick-start line, documents the `[agent.generation]` profile (noting the flat keys still work and that `[agent.work]` belongs to karet), and widens the `gh` setup step, which said PR commands were the only thing needing authentication. The bullet leads with the fallback rather than the generation, because that is the property a reader needs to trust the command: a missing, hung, or nonsensical agent degrades to a deterministic name instead of blocking the worktree. Also corrects the `agent` cargo-feature comment. It claimed the feature would gate `agent-text` "once the issue flow lands" — the issue flow lands here without it, and gating it that way would be wrong anyway: `wt issue` has to work with no agent at all. Claude-Session: https://claude.ai/code/session_014ce9P64RKbVU6kj3GmA1mo --- Cargo.toml | 7 +++++-- README.md | 37 +++++++++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9be8a8f..7d4df65 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,8 +76,11 @@ tui = [ # the current source layout; the epic's agent rework (#100) is what will let # them separate. pr = ["tui", "dep:sendit"] -# The code-agent integration (PR auto-fill). Currently gates no external -# dependency; it will gate `agent-text` once the issue flow lands. +# The code-agent integration (the PR auto-fill and the `wt issue` branch/brief +# proposal). Currently gates no external dependency; it will gate `agent-text` +# if and when the multi-provider agent migration lands. Note the issue flow is +# deliberately *not* gated on it: `wt issue` must work with no agent at all +# (issue #98), so it falls back rather than disappearing from the CLI. agent = ["cli"] [dependencies] diff --git a/README.md b/README.md index aa7f602..6006913 100644 --- a/README.md +++ b/README.md @@ -71,15 +71,15 @@ step that sets up everything; there's no separate completions install. (A static values-unaware script is still available via `wt completions ` if you want to manage it yourself.) -### 3. Authenticate `gh` (only for PR commands) +### 3. Authenticate `gh` (for PR and issue commands) ```bash gh auth login ``` -Everything except `wt pr` works fully offline. If `gh` is missing or -unauthenticated, only the PR commands fail (with an actionable message); the rest -keep working. +Everything except `wt pr` and `wt issue` works fully offline. If `gh` is missing +or unauthenticated, only those commands fail (with an actionable message); the +rest keep working. ### 4. Open it @@ -89,6 +89,7 @@ sorting, and filtering are all discoverable from there. For example: ```bash wt new feature/login # create the branch + worktree and switch into it +wt issue 123 # branch + worktree for a GitHub issue, name proposed for you wt switch # fuzzy-pick a worktree to jump to ``` @@ -109,6 +110,20 @@ These are the things worth knowing up front; the rest is discoverable from suggest existing local **and** remote branches to fork from or check out — type to filter, `↑/↓` to pick, `Enter` to accept, or just type a brand-new name. The PR compose form's model and effort fields list their choices the same way. +- **Start from a GitHub issue.** `wt issue 123` fetches the issue's title, body, + labels, type and milestone, proposes a conventional `type/123-slug` branch and a + short implementation brief, lets you edit both, then creates the worktree and + records the link. `wt list` can show it with the opt-in `issue` column + (`wt config set list.columns status,dirty,branch,issue,path`), and running + `wt issue 123` again reuses the branch it already made. + + Generation is best-effort and never blocks you: if the agent is missing, hangs, + or returns nonsense, `wt` falls back to a deterministic name built from the + issue's own labels and title, tells you it did, and carries on. Set which model + it uses under `[agent.generation]`, or per-run with `--model`/`--effort`. + + `wt` stops at the prepared worktree — it does not run a coding agent for you. + Handing the work to an agent is [karet](https://github.com/getkono/karet)'s job. - **Where worktrees are created.** New worktrees follow a configurable path template. The default keeps them beside the repo, out of it, and prefixes each worktree directory with the repo name so it's obvious which repo you're in: @@ -173,6 +188,20 @@ These are the things worth knowing up front; the rest is discoverable from the user config); precedence is flags > repo > global. `wt init` is an optional convenience that scaffolds a starter `.wt.toml` and, for a subdir store, offers to add it to `.gitignore`. +- **Pick the generation model.** The short, structured generation steps `wt` + owns — the `wt pr open --ai` draft and the `wt issue` branch/brief proposal — + read one profile: + + ```toml + [agent.generation] + provider = "claude" + model = "sonnet" # opus | sonnet | haiku + effort = "medium" # low | medium | high + ``` + + The older flat `agent.model` / `agent.effort` keys still work and mean the same + thing. `[agent.work]` is deliberately not a `wt` setting: running a coding agent + on the work belongs to [karet](https://github.com/getkono/karet). - **Theme the TUI.** Pick a built-in palette and tweak individual colors under `[ui.theme]`: `preset` selects the base (`one-dark` (default) or `solarized`), and the named slots (`accent`, `green`, `red`, `yellow`, `orange`, `cyan`, From 258f879f160af0a06d667d481ad531ed1670739c Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Fri, 28 Aug 2026 20:47:27 -0400 Subject: [PATCH 8/9] fix(create): derive the preview and the real target from one rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `preview_target` (added for `wt issue`'s confirmation prompt) derived the directory slug's fallback hash from the branch's own tip, while `create_in` derives it from the base the branch forks from. For a *new* branch the branch has no tip, so the two disagreed. It only surfaces for a branch name that slugifies to nothing — `_`, say — because the hash is consulted only as rule 5's fallback. But that is exactly where it matters: `wt issue` would confirm `repo-` and then create `repo-`. Both now go through one `target_slug`, which also subsumes the base-ref existence check `create_in` did inline, so `base_commit` is gone rather than duplicated. Two tests, because one is not enough: the first pins that preview and creation agree, but sharing a helper makes that true even if the shared rule is wrong. The second pins the rule itself — a slugless branch is named after the base short hash — and fails with `repo-` if the derivation regresses. Claude-Session: https://claude.ai/code/session_014ce9P64RKbVU6kj3GmA1mo --- src/commands/issue.rs | 4 +- src/worktree/service.rs | 100 +++++++++++++++++++++++++++++++++++----- 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/src/commands/issue.rs b/src/commands/issue.rs index 625a441..548e819 100644 --- a/src/commands/issue.rs +++ b/src/commands/issue.rs @@ -133,7 +133,7 @@ pub(crate) fn run(cx: &mut Cx, hooks: &dyn HookRunner, args: &IssueArgs) -> Resu } let env = cx.env.clone(); - let preview = preview_target(&session.parts(&env), &plan.branch)?; + let preview = preview_target(&session.parts(&env), &plan.branch, base.as_deref())?; let prompt = format!( "Issue #{}: {}\nBranch: {}\nBase: {}\nWorktree: {}\nBrief: {}\n\ Create this issue worktree? [y/N] ", @@ -517,7 +517,7 @@ fn prompt_base(cx: &mut Cx, current: Option<&str>, branches: &[String]) -> Resul /// The worktree path for `branch`, for tests that assert what was created. #[cfg(test)] fn target_of(session: &Session, env: &crate::cx::Env, branch: &str) -> std::path::PathBuf { - preview_target(&session.parts(env), branch).unwrap_or_default() + preview_target(&session.parts(env), branch, None).unwrap_or_default() } #[cfg(test)] diff --git a/src/worktree/service.rs b/src/worktree/service.rs index 54fdc5e..9cce5ba 100644 --- a/src/worktree/service.rs +++ b/src/worktree/service.rs @@ -512,13 +512,39 @@ pub(crate) fn resolve_base(repo: &Repo, config: &Config, explicit: Option<&str>) /// derives it from the same slug and template logic `create_in` uses, rather /// than reimplementing it and drifting. #[cfg(feature = "cli")] -pub(crate) fn preview_target(ws: &WorkspaceParts<'_>, branch: &str) -> Result { - let base_commit = resolve_hex(ws.repo.gix(), &branch_ref(branch)).unwrap_or_default(); - let short_hash = base_commit.get(..7).unwrap_or(&base_commit).to_string(); - let slug = slugify_with_fallback(branch, &short_hash); +pub(crate) fn preview_target( + ws: &WorkspaceParts<'_>, + branch: &str, + base: Option<&str>, +) -> Result { + let (slug, _) = target_slug(ws, branch, base)?; render_target(ws.config, ws.root, branch, &slug, ws.env) } +/// The directory slug for `branch`, derived exactly as [`create_in`] derives it. +/// +/// The commit only matters for a branch name that slugifies to nothing, where it +/// is the fallback — but that is precisely the case a preview would otherwise get +/// wrong, so the rule is shared rather than restated: an existing branch keys off +/// its own tip, a new one off the base it will fork from. +fn target_slug( + ws: &WorkspaceParts<'_>, + branch: &str, + base: Option<&str>, +) -> Result<(String, String)> { + let commit = match resolve_hex(ws.repo.gix(), &branch_ref(branch)) { + Some(oid) => oid, + None => { + let base_ref = resolve_base(ws.repo, ws.config, base).0; + resolve_hex(ws.repo.gix(), &base_ref) + .ok_or_else(|| Error::operation(format!("base ref {base_ref:?} not found")))? + } + }; + let short_hash = commit.get(..7).unwrap_or(&commit).to_string(); + let slug = slugify_with_fallback(branch, &short_hash); + Ok((slug, short_hash)) +} + /// Creates (or reuses) a worktree per `options`; see [`Workspace::create`]. pub(crate) fn create_in( ws: &WorkspaceParts<'_>, @@ -536,13 +562,10 @@ pub(crate) fn create_in( } else { Some(resolve_base(ws.repo, ws.config, options.base.as_deref()).0) }; - let base_commit = match &base_ref { - Some(base) => resolve_hex(ws.repo.gix(), base) - .ok_or_else(|| Error::operation(format!("base ref {base:?} not found")))?, - None => resolve_hex(ws.repo.gix(), &branch_ref(&branch)).unwrap_or_default(), - }; - let short_hash = base_commit.get(..7).unwrap_or(&base_commit).to_string(); - let slug = slugify_with_fallback(&branch, &short_hash); + // Shared with `preview_target`, so a confirmation preview cannot name a + // different directory than the one that gets created. This also performs the + // base-ref existence check, erroring before anything is created. + let (slug, short_hash) = target_slug(ws, &branch, options.base.as_deref())?; // If the branch is already checked out, either reuse (same target) or // refuse. The reuse path is idempotent: no copy step, no hook. @@ -1195,6 +1218,61 @@ mod tests { assert!(err.to_string().contains("already checked out")); } + // `preview_target` exists for the CLI's confirmation preview, so its tests + // only compile where it does. + #[cfg(feature = "cli")] + #[test] + fn preview_target_names_the_directory_create_actually_makes() { + // The slug's commit is only consulted for a branch name that slugifies to + // nothing — and there a *new* branch keys off the base it forks from, not + // off itself. A preview deriving the hash from the (absent) branch would + // name a different directory than the one created, so `wt issue` would + // confirm one path and produce another. + // "_" is a legal git branch name whose slug is empty, so it is the case + // that actually consults the commit hash. + for branch in ["feat/7-add-login", "_"] { + let repo = TestRepo::init(); + let ws = workspace(&repo); + let fresh = Repo::discover(repo.root()).unwrap(); + let previewed = preview_target(&ws.parts(&fresh), branch, None).unwrap(); + let created = ws + .create(&RealGit, &RealHookRunner, &create_opts(branch)) + .unwrap(); + assert_eq!( + previewed, created.path, + "preview and creation disagree for {branch:?}" + ); + } + } + + #[test] + fn a_slugless_branch_is_named_after_the_base_it_forks_from() { + // Rule 5 of the slug contract: an empty slug falls back to the short hash + // of the *base ref*, not of the branch (which does not exist yet). Pinned + // separately from the preview test, which can only prove the two agree — + // not that the rule they share is the right one. + let repo = TestRepo::init(); + let ws = workspace(&repo); + let base = Repo::discover(repo.root()) + .ok() + .and_then(|r| resolve_hex(r.gix(), "main")) + .expect("main resolves"); + let short = &base[..7]; + + let created = ws + .create(&RealGit, &RealHookRunner, &create_opts("_")) + .unwrap(); + let name = created + .path + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default(); + assert!( + name.ends_with(short), + "{name} should end with the base short hash {short}" + ); + } + #[test] fn create_with_explicit_base_records_it() { let repo = TestRepo::init(); From a05d1487715eb3aaa2daf85cdf7ea8bbe9d7c396 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Fri, 28 Aug 2026 20:59:21 -0400 Subject: [PATCH 9/9] fix(agent): return promptly when a killed agent leaves a grandchild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deadline killed the child but then joined the pipe-reader threads unconditionally. Killing a process does not close pipes a *grandchild* still holds, so the readers blocked until that grandchild exited — reinstating the full wait behind a timeout that looked like it worked. CI caught it: the test took the entire 30s. An agent CLI that is a wrapper script has exactly this shape, so this was not a theoretical case; it is the shape the timeout most needs to handle. On timeout the readers are now detached rather than joined. Their output is unwanted by definition, and they end on their own once the pipes close. The success path still joins, so stdout and stderr are unaffected. The test now uses `sleep 30 & wait` to force a grandchild. Plain `sleep 30` was too weak: a shell that `exec`s into it leaves no grandchild, which is why this passed locally and failed on CI. Claude-Session: https://claude.ai/code/session_014ce9P64RKbVU6kj3GmA1mo --- src/agent/mod.rs | 51 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/src/agent/mod.rs b/src/agent/mod.rs index a302db3..36e8365 100644 --- a/src/agent/mod.rs +++ b/src/agent/mod.rs @@ -157,18 +157,29 @@ fn run_agent( std::thread::sleep(POLL_INTERVAL); }; - // A panicking reader yields no output rather than poisoning the run. - let stdout = out_reader.join().unwrap_or_default(); - let stderr = err_reader.join().unwrap_or_default(); - match status { - Some(status) => finish(binary, status, &stdout, &stderr), - None => Err(Error::AgentTimeout { - binary: binary.to_string(), - // Sub-second deadlines still report `1s`; the message is for humans, - // and "did not respond within 0s" reads as a bug. - seconds: limit.as_secs().max(1), - }), + Some(status) => { + // A panicking reader yields no output rather than poisoning the run. + let stdout = out_reader.join().unwrap_or_default(); + let stderr = err_reader.join().unwrap_or_default(); + finish(binary, status, &stdout, &stderr) + } + None => { + // Deliberately *not* joined. Killing the child does not close the + // pipes if it left a grandchild holding them — an agent CLI that is + // a wrapper script is exactly that shape — so a reader would block + // for as long as the grandchild lives, which is precisely what the + // deadline exists to prevent. The output is unwanted anyway, so the + // readers are detached; they end on their own when the pipes close. + drop(out_reader); + drop(err_reader); + Err(Error::AgentTimeout { + binary: binary.to_string(), + // Sub-second deadlines still report `1s`; the message is for + // humans, and "did not respond within 0s" reads as a bug. + seconds: limit.as_secs().max(1), + }) + } } } @@ -354,14 +365,24 @@ mod tests { #[test] fn run_agent_kills_a_child_that_outlives_its_deadline() { - // The real defect this guards: `Command::output()` waits forever, so - // an agent that hangs used to hang `wt`. Sleeping far longer than the - // deadline proves the deadline — not the sleep — is what ends the run. + // Two defects in one test. + // + // First: `Command::output()` waits forever, so an agent that hangs + // used to hang `wt`. Sleeping far longer than the deadline proves the + // deadline — not the sleep — is what ends the run. + // + // Second, and the subtler one: `sleep 30 & wait` makes `sh` fork a + // *grandchild* that inherits the stdout/stderr pipes. Killing the + // child does not close them, so joining the reader threads blocks + // until the grandchild dies — reintroducing the full 30s wait behind + // a timeout that appears to work. An agent CLI that is a wrapper + // script has exactly this shape, so the plain `sleep 30` this test + // first used was too weak: it passes on a shell that `exec`s. let started = Instant::now(); let err = run_agent( "sh", None, - &["-c".to_string(), "sleep 30".to_string()], + &["-c".to_string(), "sleep 30 & wait".to_string()], Some(Duration::from_millis(100)), ) .unwrap_err();