90f9869a
Add implementation plan for auto-linking commits to issues
alex emery 2026-04-12 05:58
Ten-task TDD plan mapping the approved spec onto the existing codebase: event variant, IssueState surfacing with dedup, strict trailer parser, scan_and_link orchestrator, sync wiring, integration tests, CLI and TUI renderers, verification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/docs/superpowers/plans/2026-04-12-commit-issue-link.md b/docs/superpowers/plans/2026-04-12-commit-issue-link.md new file mode 100644 index 0000000..baabc0f --- /dev/null +++ b/docs/superpowers/plans/2026-04-12-commit-issue-link.md @@ -0,0 +1,1663 @@ # Auto-Link Commits to Issues Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** During `git-collab sync`, scan all local branches for commits whose messages contain an `Issue: <id>` trailer and emit idempotent `issue.commit_link` events on the matching issue DAGs, then render them under the issue in both the CLI and TUI. **Architecture:** One new event variant `Action::IssueCommitLink { commit }`. One new module `src/commit_link.rs` holding a pure trailer parser, a per-issue linked-SHA helper, and a `scan_and_link` orchestrator that walks `refs/heads/*`, dedups per-sync via `HashSet<Oid>`, and lazily reads each matched issue's event log for a cross-sync `HashMap<RefName, HashSet<String>>` dedup cache. `sync::sync` calls `scan_and_link` after `reconcile_refs("patches")` and before `collect_push_refs`, so any new link events are pushed in the same sync. `IssueState` gains a `linked_commits: Vec<LinkedCommit>` field populated by `from_ref_uncached`, which dedups by commit SHA at materialization time (the walk is topo-oldest-first, so the first-seen event wins). Both the CLI `issue show` and the TUI issue-detail widget render a new `--- Linked Commits ---` section under `--- Comments ---`. **Tech Stack:** Rust 2021, git2 0.19, serde 1, ed25519-dalek 2. No new dependencies — trailer parsing is hand-rolled string matching (no regex crate in this project). **Spec reference:** `docs/superpowers/specs/2026-04-12-commit-issue-link-design.md` --- ## File Structure | File | Role | |------|------| | `src/event.rs` | **Modify.** Add `Action::IssueCommitLink { commit }` variant. | | `src/state.rs` | **Modify.** Add `LinkedCommit` struct, `IssueState::linked_commits` field, materializer arm in `from_ref_uncached` with dedup. | | `src/commit_link.rs` | **Create.** Pure trailer parser, `collect_linked_shas`, `scan_and_link`. | | `src/lib.rs` | **Modify.** Declare the `commit_link` module. Add `--- Linked Commits ---` rendering to the `IssueCmd::Show` handler. | | `src/sync.rs` | **Modify.** Call `commit_link::scan_and_link` between `reconcile_refs("patches")` and `collect_push_refs`. | | `src/tui/widgets.rs` | **Modify.** Add `--- Linked Commits ---` section to `build_issue_detail` after comments. | | `tests/common/mod.rs` | **Modify.** Add `add_commit_link` helper mirroring `add_comment`. | | `tests/commit_link_test.rs` | **Create.** Unit tests for `parse_issue_trailers` (table-driven). | | `tests/sync_test.rs` | **Modify.** Add integration tests for the full sync→link flow. | --- ## Task 1: Add the `IssueCommitLink` event variant **Files:** - Modify: `src/event.rs` (after `Action::IssueReopen` at line 58) - Test: `tests/commit_link_test.rs` (new — start the file here) - [ ] **Step 1: Create the test file with a serde round-trip test that will fail because the variant doesn't exist yet** Create `tests/commit_link_test.rs`: ```rust //! Unit tests for src/commit_link.rs and the Action::IssueCommitLink variant. use git_collab::event::{Action, Author, Event}; fn test_author() -> Author { Author { name: "Alice".to_string(), email: "alice@example.com".to_string(), } } #[test] fn issue_commit_link_variant_round_trips() { let event = Event { timestamp: "2026-04-12T12:00:00Z".to_string(), author: test_author(), action: Action::IssueCommitLink { commit: "4b2e1cd0123456789012345678901234567890ab".to_string(), }, clock: 3, }; let json = serde_json::to_string(&event).expect("serialize"); assert!( json.contains("\"type\":\"issue.commit_link\""), "expected serde tag issue.commit_link, got: {}", json ); assert!( json.contains("\"commit\":\"4b2e1cd0123456789012345678901234567890ab\""), "expected commit field, got: {}", json ); let parsed: Event = serde_json::from_str(&json).expect("deserialize"); match parsed.action { Action::IssueCommitLink { commit } => { assert_eq!(commit, "4b2e1cd0123456789012345678901234567890ab"); } other => panic!("expected IssueCommitLink, got {:?}", other), } } ``` - [ ] **Step 2: Run the test and confirm it fails** Run: `cargo test --test commit_link_test issue_commit_link_variant_round_trips` Expected: FAIL with `no variant or associated item named 'IssueCommitLink' found for enum 'Action'` (or similar). - [ ] **Step 3: Add the variant to `src/event.rs`** Open `src/event.rs`. Locate `Action::IssueReopen` (around line 58). After it (before the `PatchCreate` variant), insert: ```rust #[serde(rename = "issue.commit_link")] IssueCommitLink { commit: String, }, ``` - [ ] **Step 4: Run the test and confirm it passes** Run: `cargo test --test commit_link_test issue_commit_link_variant_round_trips` Expected: PASS. - [ ] **Step 5: Run the full test suite to verify nothing regressed** Run: `cargo test` Expected: PASS. If any pre-existing `match event.action { ... }` now warns about a non-exhaustive pattern, that's only a warning — it's caught by the `_ => {}` arm already present at `src/state.rs:339` and `src/state.rs:551`. Don't silence warnings here; later tasks will add explicit arms where needed. - [ ] **Step 6: Commit** ```bash git add src/event.rs tests/commit_link_test.rs git commit -m "Add Action::IssueCommitLink event variant Introduces the new event kind that will be emitted during sync when a commit's message contains an Issue: trailer. Variant only; no emission sites or renderers yet." ``` --- ## Task 2: Extend `IssueState` with `linked_commits` and populate with dedup **Files:** - Modify: `src/state.rs` (struct definition around line 85, materializer around line 237) - Modify: `tests/common/mod.rs` (add `add_commit_link` helper for reuse) - Test: `tests/commit_link_test.rs` (extend with materializer tests) - [ ] **Step 1: Add the `add_commit_link` test helper** Open `tests/common/mod.rs`. After `add_comment` (around line 229), add: ```rust /// Append an IssueCommitLink event to an issue ref. Returns the new DAG tip OID. pub fn add_commit_link( repo: &Repository, ref_name: &str, author: &Author, commit_sha: &str, ) -> git2::Oid { let sk = test_signing_key(); let event = Event { timestamp: now(), author: author.clone(), action: Action::IssueCommitLink { commit: commit_sha.to_string(), }, clock: 0, }; dag::append_event(repo, ref_name, &event, &sk).unwrap() } ``` - [ ] **Step 2: Write a failing test for the materializer** Append to `tests/commit_link_test.rs`: ```rust use git2::Repository; use git_collab::state::{self, IssueState}; use tempfile::TempDir; mod common; use common::{add_commit_link, alice, init_repo, open_issue, ScopedTestConfig}; fn sha(byte: u8) -> String { format!("{:02x}{}", byte, "00".repeat(19)) } #[test] fn issue_state_surfaces_commit_links_in_order() { let _config = ScopedTestConfig::new(); let dir = TempDir::new().unwrap(); let repo = init_repo(dir.path(), &alice()); let (ref_name, id) = open_issue(&repo, &alice(), "bug"); add_commit_link(&repo, &ref_name, &alice(), &sha(0xaa)); add_commit_link(&repo, &ref_name, &alice(), &sha(0xbb)); let issue = IssueState::from_ref_uncached(&repo, &ref_name, &id).unwrap(); let commits: Vec<&str> = issue .linked_commits .iter() .map(|lc| lc.commit.as_str()) .collect(); assert_eq!(commits, vec![sha(0xaa), sha(0xbb)]); } #[test] fn issue_state_dedups_commit_links_by_sha_keeping_earliest() { let _config = ScopedTestConfig::new(); let dir = TempDir::new().unwrap(); let repo = init_repo(dir.path(), &alice()); let (ref_name, id) = open_issue(&repo, &alice(), "bug"); // Two different emitters link the same commit. First-seen should win. add_commit_link(&repo, &ref_name, &alice(), &sha(0xcc)); let bob_link = common::bob(); add_commit_link(&repo, &ref_name, &bob_link, &sha(0xcc)); let issue = IssueState::from_ref_uncached(&repo, &ref_name, &id).unwrap(); assert_eq!(issue.linked_commits.len(), 1); assert_eq!(issue.linked_commits[0].commit, sha(0xcc)); assert_eq!(issue.linked_commits[0].event_author.name, "Alice"); } ``` Note: `mod common;` inside `tests/commit_link_test.rs` references `tests/common/mod.rs` via the shared test-helper convention already used in `tests/sync_test.rs:8`. - [ ] **Step 3: Run the tests and confirm they fail** Run: `cargo test --test commit_link_test issue_state_surfaces_commit_links_in_order issue_state_dedups_commit_links_by_sha_keeping_earliest` Expected: FAIL — `IssueState` has no `linked_commits` field. - [ ] **Step 4: Add `LinkedCommit` and the `linked_commits` field** Open `src/state.rs`. After the `Comment` struct (ends around line 82), add: ```rust #[derive(Debug, Clone, Serialize, Deserialize)] pub struct LinkedCommit { /// Full 40-char commit SHA from the trailer. pub commit: String, /// Author of the `IssueCommitLink` event (who ran sync). pub event_author: Author, /// Timestamp of the `IssueCommitLink` event. pub event_timestamp: String, } ``` In the `IssueState` struct (around line 85), add a new field at the end (keep `relates_to` last is fine; add `linked_commits` just before `relates_to`): ```rust #[serde(default)] pub linked_commits: Vec<LinkedCommit>, #[serde(default, skip_serializing_if = "Option::is_none")] pub relates_to: Option<String>, ``` `#[serde(default)]` ensures older cached `IssueState` JSON (which has no `linked_commits` key) still deserializes. - [ ] **Step 5: Initialize the field in the `IssueOpen` arm of `from_ref_uncached`** In `src/state.rs:256`, locate the `Action::IssueOpen { ... } => { state = Some(IssueState { ... }); }` block and add `linked_commits: Vec::new(),` alongside the existing `comments: Vec::new(),` initialization: ```rust Action::IssueOpen { title, body, relates_to } => { state = Some(IssueState { id: id.to_string(), title, body, status: IssueStatus::Open, close_reason: None, closed_by: None, labels: Vec::new(), assignees: Vec::new(), comments: Vec::new(), linked_commits: Vec::new(), created_at: event.timestamp.clone(), last_updated: String::new(), author: event.author.clone(), relates_to, }); } ``` - [ ] **Step 6: Add an `IssueCommitLink` arm to the materializer** Still in `from_ref_uncached`, before the catchall `_ => {}` arm (around line 339), add: ```rust Action::IssueCommitLink { commit } => { if let Some(ref mut s) = state { // Render-time dedup by commit SHA. The revwalk is // topo-oldest-first (Sort::TOPOLOGICAL | Sort::REVERSE), // so the first event we see for a given SHA is the // earliest emission — exactly what the spec requires. if !s.linked_commits.iter().any(|lc| lc.commit == commit) { s.linked_commits.push(LinkedCommit { commit, event_author: event.author.clone(), event_timestamp: event.timestamp.clone(), }); } } } ``` - [ ] **Step 7: Run the tests and confirm they pass** Run: `cargo test --test commit_link_test issue_state_surfaces_commit_links_in_order issue_state_dedups_commit_links_by_sha_keeping_earliest` Expected: PASS. - [ ] **Step 8: Run the full test suite** Run: `cargo test` Expected: PASS. The `#[serde(default)]` keeps backward compatibility with cached issue state. - [ ] **Step 9: Commit** ```bash git add src/state.rs tests/common/mod.rs tests/commit_link_test.rs git commit -m "Surface linked commits on IssueState with first-seen dedup Adds LinkedCommit + IssueState.linked_commits, populated from the new event variant in IssueState::from_ref_uncached. Walking topo-oldest- first means the first-seen event for a given SHA wins, which matches the spec's render-time dedup rule. serde(default) keeps older cached state compatible." ``` --- ## Task 3: Create `src/commit_link.rs` with the trailer parser **Files:** - Create: `src/commit_link.rs` - Modify: `src/lib.rs` (module declaration) - Test: `tests/commit_link_test.rs` - [ ] **Step 1: Write the full parser test table** Append to `tests/commit_link_test.rs`: ```rust use git_collab::commit_link::parse_issue_trailers; #[test] fn parser_no_trailer_block() { assert_eq!(parse_issue_trailers("Just a plain commit"), Vec::<String>::new()); } #[test] fn parser_empty_message() { assert_eq!(parse_issue_trailers(""), Vec::<String>::new()); } #[test] fn parser_single_trailer_in_pure_block() { let msg = "Fix thing\n\nSome context in the body.\n\nIssue: abc"; assert_eq!(parse_issue_trailers(msg), vec!["abc".to_string()]); } #[test] fn parser_case_variants() { let msg1 = "subject\n\nissue: abc"; let msg2 = "subject\n\nISSUE : abc"; let msg3 = "subject\n\n Issue: abc "; assert_eq!(parse_issue_trailers(msg1), vec!["abc".to_string()]); assert_eq!(parse_issue_trailers(msg2), vec!["abc".to_string()]); assert_eq!(parse_issue_trailers(msg3), vec!["abc".to_string()]); } #[test] fn parser_two_trailers_in_pure_block() { let msg = "subject\n\nIssue: abc\nIssue: def"; assert_eq!(parse_issue_trailers(msg), vec!["abc".to_string(), "def".to_string()]); } #[test] fn parser_issue_in_body_but_not_final_paragraph() { let msg = "subject\n\nIssue: abc\n\nSigned-off-by: alice <a@example.com>"; // The final paragraph is the signed-off-by block, not the issue line. // It's a valid trailer block (Signed-off-by is trailer-shaped), but it // contains no Issue: key, so we extract nothing. assert_eq!(parse_issue_trailers(msg), Vec::<String>::new()); } #[test] fn parser_wrong_key() { let msg = "subject\n\nIssues: abc"; assert_eq!(parse_issue_trailers(msg), Vec::<String>::new()); } #[test] fn parser_prose_mention() { let msg = "subject\n\nthis fixes issue abc in the body"; assert_eq!(parse_issue_trailers(msg), Vec::<String>::new()); } #[test] fn parser_single_paragraph_whole_message_is_trailer_block() { let msg = "Issue: abc"; assert_eq!(parse_issue_trailers(msg), vec!["abc".to_string()]); } #[test] fn parser_mixed_final_paragraph_rejects_all() { let msg = "subject\n\nThanks to Bob for the catch.\nIssue: a3f9"; // Final paragraph has a prose line, so it's not a trailer block and we // extract nothing. This is the "false positive in prose" guard. assert_eq!(parse_issue_trailers(msg), Vec::<String>::new()); } #[test] fn parser_trailing_whitespace_paragraph_does_not_shadow_trailer_block() { // The final paragraph is empty/whitespace, so the walk should fall back // to the previous non-empty paragraph, which is a valid trailer block. let msg = "subject\n\nIssue: abc\n\n \n"; assert_eq!(parse_issue_trailers(msg), vec!["abc".to_string()]); } #[test] fn parser_pure_block_with_mixed_keys() { let msg = "subject\n\nSigned-off-by: alice <a@example.com>\nIssue: abc"; assert_eq!(parse_issue_trailers(msg), vec!["abc".to_string()]); } #[test] fn parser_rejects_value_with_trailing_garbage() { let msg = "subject\n\nIssue: abc fixes thing"; assert_eq!(parse_issue_trailers(msg), Vec::<String>::new()); } #[test] fn parser_rejects_empty_value() { let msg = "subject\n\nIssue: "; assert_eq!(parse_issue_trailers(msg), Vec::<String>::new()); } ``` - [ ] **Step 2: Run the tests to confirm they all fail** Run: `cargo test --test commit_link_test parser_` Expected: FAIL — `parse_issue_trailers` and the `commit_link` module don't exist. - [ ] **Step 3: Create `src/commit_link.rs` with the parser** Create `src/commit_link.rs`: ```rust //! Auto-link commits to issues via `Issue:` git trailers during sync. //! //! See: docs/superpowers/specs/2026-04-12-commit-issue-link-design.md /// Parse `Issue:` trailers from a commit message. /// /// Returns the list of trailer values in order of appearance. Follows git's /// own trailer-block semantics: only the final paragraph is considered, and /// *every* non-empty line in it must be trailer-shaped (a `token: value` /// line) for the paragraph to qualify. Any prose line in the final paragraph /// disqualifies the whole paragraph — this prevents false positives like /// `"Thanks Bob.\nIssue: abc"` in commit bodies. /// /// The key match is `(?i)issue`; the value must be a single non-whitespace /// token followed by optional trailing whitespace and end-of-line. Values /// like `abc fixes thing` are rejected so that loose commentary never /// becomes a silent issue-prefix lookup that warns every sync forever. pub fn parse_issue_trailers(message: &str) -> Vec<String> { // 1. Split into paragraphs (blank-line separated), preserving order. // Trim trailing whitespace from each line for the trailer-shape check, // but keep enough structure to recognize blank lines. let lines: Vec<&str> = message.lines().collect(); // 2. Find the last paragraph: the longest tail slice that contains at // least one non-empty line and has no blank line *before* its first // non-empty line in the tail. // // Walking from the end: skip trailing blank/whitespace-only lines, // then collect lines until we hit a blank line. let mut end = lines.len(); while end > 0 && lines[end - 1].trim().is_empty() { end -= 1; } if end == 0 { return Vec::new(); } let mut start = end; while start > 0 && !lines[start - 1].trim().is_empty() { start -= 1; } let paragraph = &lines[start..end]; // 3. Validate every non-empty line in the paragraph is trailer-shaped. for line in paragraph { if line.trim().is_empty() { continue; } if !is_trailer_shaped(line) { return Vec::new(); } } // 4. Extract `Issue:` values. let mut out = Vec::new(); for line in paragraph { if let Some(value) = match_issue_line(line) { out.push(value); } } out } /// Returns true if a line looks like a git trailer: `<token>: <value>`, where /// token starts with a letter and consists of `[A-Za-z0-9-]`, and value is at /// least one non-whitespace character. fn is_trailer_shaped(line: &str) -> bool { let trimmed = line.trim_start(); let Some(colon_pos) = trimmed.find(':') else { return false; }; let token = &trimmed[..colon_pos]; if token.is_empty() { return false; } let mut chars = token.chars(); let first = chars.next().unwrap(); if !first.is_ascii_alphabetic() { return false; } if !chars.all(|c| c.is_ascii_alphanumeric() || c == '-') { return false; } let value = trimmed[colon_pos + 1..].trim(); !value.is_empty() } /// If `line` is an `Issue: <token>` trailer with exactly one non-whitespace /// token in its value, returns the token. Otherwise returns None. fn match_issue_line(line: &str) -> Option<String> { let trimmed = line.trim_start(); let colon_pos = trimmed.find(':')?; let key = trimmed[..colon_pos].trim_end(); if !key.eq_ignore_ascii_case("issue") { return None; } let value_region = &trimmed[colon_pos + 1..]; let value = value_region.trim(); if value.is_empty() { return None; } // Reject values with interior whitespace: `abc fixes thing` must not // parse to `abc` silently — it must parse to nothing so the user sees // that their commentary is being ignored. if value.split_whitespace().count() != 1 { return None; } Some(value.to_string()) } ``` - [ ] **Step 4: Declare the module in `src/lib.rs`** Open `src/lib.rs`. At the top of the module declarations (lines 1–17), insert alphabetically after `pub mod cli;`: ```rust pub mod commit_link; ``` Final ordering should be: ```rust pub mod cache; pub mod cli; pub mod commit_link; pub mod dag; pub mod editor; ... ``` - [ ] **Step 5: Run the parser tests and confirm all pass** Run: `cargo test --test commit_link_test parser_` Expected: PASS for all 14 parser tests. - [ ] **Step 6: Run clippy on the new module** Run: `cargo clippy --all-targets -- -D warnings` Expected: no warnings. - [ ] **Step 7: Commit** ```bash git add src/commit_link.rs src/lib.rs tests/commit_link_test.rs git commit -m "Add src/commit_link.rs with strict Issue: trailer parser Implements the spec's strict trailer-block semantics: the final paragraph qualifies only if every non-empty line is trailer-shaped, which rejects prose false positives like 'Thanks Bob.\\nIssue: a3f9'. The Issue: value must be a single token, so loose commentary like 'Issue: abc fixes thing' is also rejected. Pure parser, no git or I/O, 14 table-driven unit tests." ``` --- ## Task 4: Add `collect_linked_shas` helper **Files:** - Modify: `src/commit_link.rs` - Test: `tests/commit_link_test.rs` - [ ] **Step 1: Write a failing test** Append to `tests/commit_link_test.rs`: ```rust use git_collab::commit_link::collect_linked_shas; #[test] fn collect_linked_shas_empty_for_fresh_issue() { let _config = ScopedTestConfig::new(); let dir = TempDir::new().unwrap(); let repo = init_repo(dir.path(), &alice()); let (ref_name, _id) = open_issue(&repo, &alice(), "bug"); let shas = collect_linked_shas(&repo, &ref_name).unwrap(); assert!(shas.is_empty()); } #[test] fn collect_linked_shas_returns_all_linked_commits_including_duplicates() { let _config = ScopedTestConfig::new(); let dir = TempDir::new().unwrap(); let repo = init_repo(dir.path(), &alice()); let (ref_name, _id) = open_issue(&repo, &alice(), "bug"); add_commit_link(&repo, &ref_name, &alice(), &sha(0xaa)); add_commit_link(&repo, &ref_name, &alice(), &sha(0xbb)); // Even a duplicate DAG entry (cross-machine race) is surfaced here — // this is the "source of truth" for whether we need to emit. add_commit_link(&repo, &ref_name, &alice(), &sha(0xaa)); let shas = collect_linked_shas(&repo, &ref_name).unwrap(); assert_eq!(shas.len(), 2); assert!(shas.contains(&sha(0xaa))); assert!(shas.contains(&sha(0xbb))); } ``` - [ ] **Step 2: Run the tests and confirm they fail** Run: `cargo test --test commit_link_test collect_linked_shas_` Expected: FAIL — `collect_linked_shas` is not defined. - [ ] **Step 3: Add `collect_linked_shas` to `src/commit_link.rs`** Append to `src/commit_link.rs`: ```rust use std::collections::HashSet; use git2::Repository; use crate::dag; use crate::error::Error; use crate::event::Action; /// Walk an issue's event DAG and return every commit SHA that has an /// `IssueCommitLink` event attached. Called lazily on first match per issue /// during `scan_and_link`; the result is cached in the orchestrator's /// `HashMap<RefName, HashSet<String>>`. pub fn collect_linked_shas(repo: &Repository, issue_ref: &str) -> Result<HashSet<String>, Error> { let events = dag::walk_events(repo, issue_ref)?; let mut shas = HashSet::new(); for (_oid, event) in events { if let Action::IssueCommitLink { commit } = event.action { shas.insert(commit); } } Ok(shas) } ``` - [ ] **Step 4: Run the tests and confirm they pass** Run: `cargo test --test commit_link_test collect_linked_shas_` Expected: PASS. - [ ] **Step 5: Commit** ```bash git add src/commit_link.rs tests/commit_link_test.rs git commit -m "Add collect_linked_shas helper to commit_link module Walks an issue's DAG and returns every SHA appearing in an IssueCommitLink event. Used by scan_and_link as the per-issue dedup source of truth, cached on first match per sync." ``` --- ## Task 5: Implement `scan_and_link` orchestrator with a happy-path integration test **Files:** - Modify: `src/commit_link.rs` - Test: `tests/sync_test.rs` (add the first integration test) - [ ] **Step 1: Write the happy-path integration test** Open `tests/sync_test.rs`. Find the end of the existing test module (near line 1200+) and append a new section: ```rust // --------------------------------------------------------------------------- // Commit-link tests (src/commit_link.rs) // --------------------------------------------------------------------------- use git_collab::commit_link; fn make_commit_with_message(cluster: &TestCluster, repo: &Repository, message: &str) -> git2::Oid { let _ = cluster; // silence unused if not needed let sig = git2::Signature::now("Alice", "alice@example.com").unwrap(); let head = repo.head().unwrap(); let parent_oid = head.target().unwrap(); let parent = repo.find_commit(parent_oid).unwrap(); let tree_oid = parent.tree().unwrap().id(); let tree = repo.find_tree(tree_oid).unwrap(); // Commit on current branch (refs/heads/main). repo.commit(Some("HEAD"), &sig, &sig, message, &tree, &[&parent]) .unwrap() } #[test] fn commit_link_scan_emits_event_for_matching_trailer() { let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); // Open an issue. let (issue_ref, issue_id) = open_issue(&alice_repo, &alice(), "fix the walker"); // Create a commit whose trailer references that issue. let message = format!("Fix walker\n\nIssue: {}", &issue_id[..8]); let commit_oid = make_commit_with_message(&cluster, &alice_repo, &message); // Run the scanner directly (we test the sync integration in later tests). let author = git_collab::identity::get_author(&alice_repo).unwrap(); let sk = signing::load_signing_key( &signing::signing_key_dir().unwrap(), ) .unwrap(); let emitted = commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(); assert_eq!(emitted, 1); // Walk the issue's event log and find the link. let issue = IssueState::from_ref_uncached(&alice_repo, &issue_ref, &issue_id).unwrap(); assert_eq!(issue.linked_commits.len(), 1); assert_eq!(issue.linked_commits[0].commit, commit_oid.to_string()); } ``` - [ ] **Step 2: Run the test and confirm it fails** Run: `cargo test --test sync_test commit_link_scan_emits_event_for_matching_trailer` Expected: FAIL — `scan_and_link` is not defined. - [ ] **Step 3: Add `scan_and_link` to `src/commit_link.rs`** Append to `src/commit_link.rs`: ```rust use std::collections::HashMap; use git2::{Oid, Sort}; use crate::event::{Author, Event}; const ACTIVE_ISSUE_PREFIX: &str = "refs/collab/issues/"; const ARCHIVED_ISSUE_PREFIX: &str = "refs/collab/archive/issues/"; /// Walk every commit reachable from `refs/heads/*`, parse `Issue:` trailers, /// resolve each to an issue, and emit an `IssueCommitLink` event for any /// (issue, commit) pair that doesn't already have one. /// /// **Never breaks sync.** Per-commit and per-issue errors are logged as /// one-line stderr warnings and iteration continues. The only errors that /// propagate are "couldn't even start" failures (opening the repo, building /// the revwalk). Callers treat a returned `Err` as "skip the link scan for /// this sync" and proceed. /// /// Returns the number of events actually emitted. pub fn scan_and_link( repo: &Repository, author: &Author, sk: &ed25519_dalek::SigningKey, ) -> Result<usize, Error> { // Build a revwalk seeded from every local branch tip. let mut revwalk = repo.revwalk()?; revwalk.set_sorting(Sort::TOPOLOGICAL)?; let mut seeded_any = false; for reference in repo.references_glob("refs/heads/*")? { let Ok(reference) = reference else { continue }; let Some(target) = reference.target() else { continue }; // `revwalk.push` dedups commits across branch tips internally. if revwalk.push(target).is_ok() { seeded_any = true; } } if !seeded_any { // Detached HEAD with no local branches. Silent no-op per spec. return Ok(0); } // Per-sync dedup of commits already visited. let mut visited: HashSet<Oid> = HashSet::new(); // Cache of existing link SHAs per resolved issue ref. `None` = poisoned. let mut link_cache: HashMap<String, Option<HashSet<String>>> = HashMap::new(); let mut emitted: usize = 0; for oid_result in revwalk { let oid = match oid_result { Ok(o) => o, Err(e) => { eprintln!("warning: revwalk error, stopping scan: {}", e); break; } }; if !visited.insert(oid) { continue; } let commit = match repo.find_commit(oid) { Ok(c) => c, Err(e) => { eprintln!("warning: cannot load commit {}: {}", oid, e); continue; } }; let message = commit.message().unwrap_or(""); let trailers = parse_issue_trailers(message); if trailers.is_empty() { continue; } for prefix in trailers { match crate::state::resolve_issue_ref(repo, &prefix) { Ok((resolved_ref, _resolved_id)) => { if resolved_ref.starts_with(ARCHIVED_ISSUE_PREFIX) { eprintln!( "warning: commit {}: Issue: {} — issue is archived, skipping", oid, prefix ); continue; } if !resolved_ref.starts_with(ACTIVE_ISSUE_PREFIX) { // Unknown namespace. Should not happen with current // resolver, but belt-and-braces. eprintln!( "warning: commit {}: Issue: {} — resolved to unexpected ref {}, skipping", oid, prefix, resolved_ref ); continue; } let entry = link_cache.entry(resolved_ref.clone()).or_insert_with(|| { match collect_linked_shas(repo, &resolved_ref) { Ok(set) => Some(set), Err(e) => { eprintln!( "warning: cannot read link events for {}: {} — skipping issue for the rest of this sync", resolved_ref, e ); None } } }); let Some(ref mut set) = entry else { continue }; let sha = oid.to_string(); if set.contains(&sha) { continue; } let event = Event { timestamp: chrono::Utc::now().to_rfc3339(), author: author.clone(), action: Action::IssueCommitLink { commit: sha.clone(), }, clock: 0, }; match dag::append_event(repo, &resolved_ref, &event, sk) { Ok(_) => { set.insert(sha); emitted += 1; } Err(e) => { eprintln!( "warning: failed to emit IssueCommitLink on {}: {}", resolved_ref, e ); } } } Err(e) => { // resolve_issue_ref error message already distinguishes // "no issue found" from "ambiguous prefix". eprintln!( "warning: commit {}: Issue: {} — {}, skipping", oid, prefix, e ); } } } } Ok(emitted) } ``` - [ ] **Step 4: Run the integration test and confirm it passes** Run: `cargo test --test sync_test commit_link_scan_emits_event_for_matching_trailer` Expected: PASS. The test opens an issue, writes a commit with `Issue: <prefix>`, calls `scan_and_link` directly, and confirms the link event was appended. - [ ] **Step 5: Run clippy** Run: `cargo clippy --all-targets -- -D warnings` Expected: no warnings. - [ ] **Step 6: Commit** ```bash git add src/commit_link.rs tests/sync_test.rs git commit -m "Implement commit_link::scan_and_link orchestrator Walks refs/heads/* (dedup'd via HashSet<Oid>), parses Issue: trailers from each commit, resolves to an issue ref, and appends IssueCommitLink events. Per-sync cache keyed by resolved ref name absorbs repeat matches; all per-commit and per-issue errors become stderr warnings so the scan never breaks sync. Archived issues are skipped with a warning." ``` --- ## Task 6: Wire `scan_and_link` into `sync::sync` **Files:** - Modify: `src/sync.rs` (after `reconcile_refs("patches", ...)` at line 365) - Modify: `tests/sync_test.rs` (confirm the wiring runs end-to-end via `sync::sync`) - [ ] **Step 1: Write an end-to-end test driving the full `sync::sync` entry point** Append to `tests/sync_test.rs` below the existing commit-link section: ```rust #[test] fn sync_entry_point_emits_commit_link_events_and_pushes_them() { let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); // Alice opens an issue and syncs it up so Bob will see it too. let (_issue_ref, issue_id) = open_issue(&alice_repo, &alice(), "bug"); sync::sync(&alice_repo, "origin").unwrap(); // Alice makes a real commit with an Issue: trailer. let message = format!("Fix the thing\n\nIssue: {}", &issue_id[..8]); make_commit_with_message(&cluster, &alice_repo, &message); // Running sync should scan, emit the link, and push it. sync::sync(&alice_repo, "origin").unwrap(); // Bob fetches and should see the link in the materialized issue. let bob_repo = cluster.bob_repo(); sync::sync(&bob_repo, "origin").unwrap(); let bob_issue_ref = format!("refs/collab/issues/{}", issue_id); let bob_issue = IssueState::from_ref_uncached(&bob_repo, &bob_issue_ref, &issue_id).unwrap(); assert_eq!(bob_issue.linked_commits.len(), 1); } ``` - [ ] **Step 2: Run the test and confirm it fails** Run: `cargo test --test sync_test sync_entry_point_emits_commit_link_events_and_pushes_them` Expected: FAIL — `sync::sync` doesn't call `scan_and_link` yet, so no link events exist for Bob to fetch. - [ ] **Step 3: Wire the call into `src/sync.rs`** Open `src/sync.rs`. Find the block around line 362–366: ```rust let repo = Repository::open(repo.path())?; let sk = signing::load_signing_key(&signing::signing_key_dir()?)?; reconcile_refs(&repo, "issues", &author, &sk)?; reconcile_refs(&repo, "patches", &author, &sk)?; // Step 3: Push collab refs individually ``` Insert the scan call between `reconcile_refs("patches", ...)` and `// Step 3: Push collab refs individually`: ```rust let repo = Repository::open(repo.path())?; let sk = signing::load_signing_key(&signing::signing_key_dir()?)?; reconcile_refs(&repo, "issues", &author, &sk)?; reconcile_refs(&repo, "patches", &author, &sk)?; // Step 2.5: Scan local branches for Issue: trailers and emit link events. // Never breaks sync — scan_and_link absorbs per-commit/per-issue errors. match crate::commit_link::scan_and_link(&repo, &author, &sk) { Ok(n) if n > 0 => println!("Linked {} commit(s) to issues.", n), Ok(_) => {} Err(e) => eprintln!("warning: commit link scan failed: {}", e), } // Step 3: Push collab refs individually ``` - [ ] **Step 4: Add the missing `crate::commit_link` import (if clippy wants it)** `crate::commit_link::scan_and_link` is fully qualified in the call above, so no `use` is needed. Leave as-is. - [ ] **Step 5: Run the integration test and confirm it passes** Run: `cargo test --test sync_test sync_entry_point_emits_commit_link_events_and_pushes_them` Expected: PASS. - [ ] **Step 6: Run the full sync test suite** Run: `cargo test --test sync_test` Expected: PASS. Existing sync tests should be unaffected because no existing test creates commits with `Issue:` trailers. - [ ] **Step 7: Commit** ```bash git add src/sync.rs tests/sync_test.rs git commit -m "Wire commit_link::scan_and_link into the sync pipeline Called after reconcile_refs so the dedup cache reflects merged remote state, and before collect_push_refs so new link events ride out in the same sync. End-to-end test: Alice writes a commit with an Issue: trailer, sync emits+pushes, Bob fetches and materializes the linked commit on the issue." ``` --- ## Task 7: Add the remaining integration tests **Files:** - Modify: `tests/sync_test.rs` - [ ] **Step 1: Add idempotency and multi-branch tests** Append to `tests/sync_test.rs`: ```rust #[test] fn commit_link_scan_is_idempotent_across_runs() { let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); let (issue_ref, issue_id) = open_issue(&alice_repo, &alice(), "bug"); let message = format!("Fix thing\n\nIssue: {}", &issue_id[..8]); make_commit_with_message(&cluster, &alice_repo, &message); let author = git_collab::identity::get_author(&alice_repo).unwrap(); let sk = signing::load_signing_key(&signing::signing_key_dir().unwrap()).unwrap(); assert_eq!(commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(), 1); assert_eq!(commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(), 0); assert_eq!(commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(), 0); let issue = IssueState::from_ref_uncached(&alice_repo, &issue_ref, &issue_id).unwrap(); assert_eq!(issue.linked_commits.len(), 1); } #[test] fn commit_link_scan_walks_all_local_branches_and_dedups_shared_ancestors() { let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); let (issue_ref, issue_id) = open_issue(&alice_repo, &alice(), "bug"); // Commit on main with the trailer. Both branches will reach it. let message = format!("Fix\n\nIssue: {}", &issue_id[..8]); let linked_commit = make_commit_with_message(&cluster, &alice_repo, &message); // Create a second branch pointing at the same commit. { let commit = alice_repo.find_commit(linked_commit).unwrap(); alice_repo.branch("feature-x", &commit, false).unwrap(); } let author = git_collab::identity::get_author(&alice_repo).unwrap(); let sk = signing::load_signing_key(&signing::signing_key_dir().unwrap()).unwrap(); // Should emit exactly one event despite the commit being reachable from // two branch tips. assert_eq!(commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(), 1); let issue = IssueState::from_ref_uncached(&alice_repo, &issue_ref, &issue_id).unwrap(); assert_eq!(issue.linked_commits.len(), 1); } #[test] fn commit_link_scan_handles_multiple_issue_trailers_on_one_commit() { let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); let (issue_ref_a, id_a) = open_issue(&alice_repo, &alice(), "bug a"); let (issue_ref_b, id_b) = open_issue(&alice_repo, &alice(), "bug b"); let message = format!( "Fix both\n\nIssue: {}\nIssue: {}", &id_a[..8], &id_b[..8] ); make_commit_with_message(&cluster, &alice_repo, &message); let author = git_collab::identity::get_author(&alice_repo).unwrap(); let sk = signing::load_signing_key(&signing::signing_key_dir().unwrap()).unwrap(); assert_eq!(commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(), 2); let issue_a = IssueState::from_ref_uncached(&alice_repo, &issue_ref_a, &id_a).unwrap(); let issue_b = IssueState::from_ref_uncached(&alice_repo, &issue_ref_b, &id_b).unwrap(); assert_eq!(issue_a.linked_commits.len(), 1); assert_eq!(issue_b.linked_commits.len(), 1); } ``` - [ ] **Step 2: Run these three tests** Run: `cargo test --test sync_test commit_link_scan_is_idempotent commit_link_scan_walks_all commit_link_scan_handles_multiple` Expected: PASS. - [ ] **Step 3: Add unknown/ambiguous/archived prefix tests** Append to `tests/sync_test.rs`: ```rust #[test] fn commit_link_scan_skips_unknown_prefix_without_error() { let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); // No issue exists. Commit uses a completely unrelated prefix. make_commit_with_message( &cluster, &alice_repo, "Fix\n\nIssue: zzzzzzzz", ); let author = git_collab::identity::get_author(&alice_repo).unwrap(); let sk = signing::load_signing_key(&signing::signing_key_dir().unwrap()).unwrap(); assert_eq!(commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(), 0); } #[test] fn commit_link_scan_skips_ambiguous_prefix_without_error() { let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); // Open two issues. Their OIDs are random, so we can't guarantee they // share a prefix — instead, use a single-character prefix that is // likely to be ambiguous. If the two OIDs happen to not share a first // char, this test degrades to a no-op but still passes the "no error" // check, which is the important property. let (_, id_a) = open_issue(&alice_repo, &alice(), "a"); let (_, id_b) = open_issue(&alice_repo, &alice(), "b"); // Find a shared character prefix, or fall back to the first char of a. let shared_prefix = if id_a.chars().next() == id_b.chars().next() { id_a[..1].to_string() } else { // No ambiguity possible — use a prefix that matches only a, which // exercises the resolve-success path instead. Test name still holds // because "without error" is the core assertion. id_a[..8].to_string() }; make_commit_with_message( &cluster, &alice_repo, &format!("Touch\n\nIssue: {}", shared_prefix), ); let author = git_collab::identity::get_author(&alice_repo).unwrap(); let sk = signing::load_signing_key(&signing::signing_key_dir().unwrap()).unwrap(); // Must not error regardless of whether the prefix ambiguously matched. let _ = commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(); } #[test] fn commit_link_scan_skips_archived_issues_with_warning() { let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); let (_, issue_id) = open_issue(&alice_repo, &alice(), "old bug"); // Archive the issue via the state helper. state::archive_issue_ref(&alice_repo, &issue_id).unwrap(); make_commit_with_message( &cluster, &alice_repo, &format!("Reference old\n\nIssue: {}", &issue_id[..8]), ); let author = git_collab::identity::get_author(&alice_repo).unwrap(); let sk = signing::load_signing_key(&signing::signing_key_dir().unwrap()).unwrap(); assert_eq!(commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(), 0); // Confirm the archived ref did not accrue a new event: the archived // DAG tip should still be the archive-time tip. let archived_ref = format!("refs/collab/archive/issues/{}", issue_id); let archived_state = IssueState::from_ref_uncached(&alice_repo, &archived_ref, &issue_id).unwrap(); assert!(archived_state.linked_commits.is_empty()); } ``` - [ ] **Step 4: Run these tests** Run: `cargo test --test sync_test commit_link_scan_skips_unknown commit_link_scan_skips_ambiguous commit_link_scan_skips_archived` Expected: PASS. Warning output on stderr is expected and does not fail the test. - [ ] **Step 5: Add detached-HEAD and remote-originated dedup tests** Append to `tests/sync_test.rs`: ```rust #[test] fn commit_link_scan_no_op_on_detached_head_with_no_branches() { let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); // Delete all local branches and put HEAD in detached state. let head_oid = alice_repo.head().unwrap().target().unwrap(); alice_repo.set_head_detached(head_oid).unwrap(); // Remove refs/heads/main. alice_repo.find_reference("refs/heads/main").unwrap().delete().unwrap(); let author = git_collab::identity::get_author(&alice_repo).unwrap(); let sk = signing::load_signing_key(&signing::signing_key_dir().unwrap()).unwrap(); // No branches to walk — silent no-op. assert_eq!(commit_link::scan_and_link(&alice_repo, &author, &sk).unwrap(), 0); } #[test] fn commit_link_scan_dedups_against_remote_originated_events() { // Simulates the cross-machine dedup case: Bob's repo fetches a link // event that Alice already emitted, then runs scan locally with the // same commit reachable from his branches. He must not emit a // duplicate. let cluster = TestCluster::new(); let alice_repo = cluster.alice_repo(); let bob_repo = cluster.bob_repo(); // Both repos get the same issue. let (_, issue_id) = open_issue(&alice_repo, &alice(), "bug"); sync::sync(&alice_repo, "origin").unwrap(); sync::sync(&bob_repo, "origin").unwrap(); // Alice writes a commit and pushes it to the bare remote so Bob can // fetch it. First the regular git push; then sync for the link event. let message = format!("Fix thing\n\nIssue: {}", &issue_id[..8]); let linked_commit = make_commit_with_message(&cluster, &alice_repo, &message); // Push the branch so Bob sees the commit too. let mut cmd = Command::new("git"); cmd.args(["push", "origin", "main"]) .current_dir(cluster.alice_dir.path()); assert!(cmd.status().unwrap().success()); sync::sync(&alice_repo, "origin").unwrap(); // Bob fetches both the branch and the collab link event. let mut cmd = Command::new("git"); cmd.args(["fetch", "origin", "main:main"]) .current_dir(cluster.bob_dir.path()); assert!(cmd.status().unwrap().success()); sync::sync(&bob_repo, "origin").unwrap(); // At this point Bob's issue already has the link event from Alice. // Re-running scan on Bob's repo must find the commit locally and // decide "already linked", emitting zero events. let author = git_collab::identity::get_author(&bob_repo).unwrap(); let sk = signing::load_signing_key(&signing::signing_key_dir().unwrap()).unwrap(); let emitted = commit_link::scan_and_link(&bob_repo, &author, &sk).unwrap(); assert_eq!(emitted, 0, "Bob must not duplicate Alice's link event"); // And the commit on Bob's side really is the one linked. let bob_ref = format!("refs/collab/issues/{}", issue_id); let bob_issue = IssueState::from_ref_uncached(&bob_repo, &bob_ref, &issue_id).unwrap(); assert_eq!(bob_issue.linked_commits.len(), 1); assert_eq!(bob_issue.linked_commits[0].commit, linked_commit.to_string()); } ``` - [ ] **Step 6: Run these tests** Run: `cargo test --test sync_test commit_link_scan_no_op_on_detached commit_link_scan_dedups_against_remote` Expected: PASS. - [ ] **Step 7: Run the entire sync test suite as a regression check** Run: `cargo test --test sync_test` Expected: PASS. - [ ] **Step 8: Commit** ```bash git add tests/sync_test.rs git commit -m "Add integration tests for commit-link scan edge cases Covers idempotency, multi-branch ancestor dedup, multi-issue trailers, unknown/ambiguous/archived prefixes, detached HEAD silent no-op, and cross-machine dedup against remote-originated link events." ``` --- ## Task 8: Render linked commits in the CLI `issue show` output **Files:** - Modify: `src/lib.rs` (the `IssueCmd::Show` match arm, around lines 115–148) - Test: `tests/cli_test.rs` or add a new CLI-level assertion in `sync_test.rs` — we'll extend `sync_test.rs` for co-location with other commit-link tests. - [ ] **Step 1: Write a failing test that uses the CLI to show an issue with a link** Append to `tests/sync_test.rs`. Note: this uses `TestRepo` from `common`, which runs the binary — so it's a process-level test. ```rust #[test] fn cli_issue_show_renders_linked_commits_section() { use common::TestRepo; let repo = TestRepo::new("Alice", "alice@example.com"); let issue_id = repo.issue_open("fix the thing"); // Write a commit with an Issue: trailer via git CLI. let full_id = { let out = repo.run_ok(&["issue", "show", &issue_id, "--json"]); let v: serde_json::Value = serde_json::from_str(&out).unwrap(); v["id"].as_str().unwrap().to_string() }; let msg = format!("Fix a thing\n\nIssue: {}", &full_id[..8]); repo.git(&["commit", "--allow-empty", "-m", &msg]); // Run sync against a bare remote we create inline. let bare = TempDir::new().unwrap(); Command::new("git") .args(["init", "--bare"]) .current_dir(bare.path()) .status() .unwrap(); repo.git(&["remote", "add", "origin", bare.path().to_str().unwrap()]); repo.git(&["push", "-u", "origin", "main"]); repo.run_ok(&["init"]); repo.run_ok(&["sync"]); let show = repo.run_ok(&["issue", "show", &issue_id]); assert!( show.contains("--- Linked Commits ---"), "expected linked commits section, got:\n{}", show ); assert!( show.contains("by Alice"), "expected commit author rendered, got:\n{}", show ); assert!( show.contains("(linked by Alice"), "expected event author rendered, got:\n{}", show ); } ``` - [ ] **Step 2: Run the test and confirm it fails** Run: `cargo test --test sync_test cli_issue_show_renders_linked_commits_section` Expected: FAIL — the renderer does not yet output the `--- Linked Commits ---` section. - [ ] **Step 3: Add the renderer in `src/lib.rs`** Open `src/lib.rs`. Find the `IssueCmd::Show` arm (starts around line 115). Locate the block that renders comments: ```rust if !i.comments.is_empty() { println!("\n--- Comments ---"); for c in &i.comments { println!("\n{} ({}):\n{}", c.author.name, c.timestamp, c.body); } } Ok(()) ``` Immediately before `Ok(())`, add the linked-commits renderer: ```rust if !i.linked_commits.is_empty() { println!("\n--- Linked Commits ---"); for lc in &i.linked_commits { let short_sha = if lc.commit.len() >= 7 { &lc.commit[..7] } else { &lc.commit }; let (subject, commit_author) = match git2::Oid::from_str(&lc.commit) .ok() .and_then(|oid| repo.find_commit(oid).ok()) { Some(commit) => { let subject = commit .summary() .map(|s| truncate_summary(s, 60)) .unwrap_or_default(); let author = commit .author() .name() .unwrap_or("unknown") .to_string(); (Some(subject), Some(author)) } None => (None, None), }; match (subject, commit_author) { (Some(subject), Some(author)) => { println!( "· linked {} \"{}\" by {} (linked by {}, {})", short_sha, subject, author, lc.event_author.name, lc.event_timestamp, ); } _ => { println!( "· linked {} (commit {} not in local repo) (linked by {}, {})", short_sha, short_sha, lc.event_author.name, lc.event_timestamp, ); } } } } Ok(()) ``` Add a helper at the bottom of `src/lib.rs` (outside all `fn run` code, next to `fn search`): ```rust fn truncate_summary(s: &str, max_chars: usize) -> String { let mut out = String::new(); let mut count = 0; for c in s.chars() { if count + 1 > max_chars { out.push('…'); return out; } out.push(c); count += 1; } out } ``` - [ ] **Step 4: Run the test and confirm it passes** Run: `cargo test --test sync_test cli_issue_show_renders_linked_commits_section` Expected: PASS. - [ ] **Step 5: Run the full CLI test suite** Run: `cargo test` Expected: PASS. No existing `issue show` tests should break because they use issues without any linked commits — the new section is only printed when non-empty. - [ ] **Step 6: Commit** ```bash git add src/lib.rs tests/sync_test.rs git commit -m "Render --- Linked Commits --- section in CLI issue show Each line shows the short SHA, commit subject (truncated to 60 chars), commit author, and then (linked by <event-author>, <timestamp>). When the commit isn't locally reachable (GC'd, shallow), falls back to a no-subject variant that still shows the linked-by metadata." ``` --- ## Task 9: Render linked commits in the TUI issue detail widget **Files:** - Modify: `src/tui/widgets.rs` (function rendering issue detail, around lines 390–464) - Test: Extend an existing TUI smoke test or rely on the existing dashboard smoke harness — the widget is ultimately driven by `IssueState`, which already carries `linked_commits`, so we'll test via a direct widget call. - [ ] **Step 1: Locate the relevant widget function** Run: `cargo clippy --all-targets 2>&1 | head -5` — just to confirm build is clean before editing. Then open `src/tui/widgets.rs` and read the function that builds issue detail (search for `--- Comments ---` — it's the same function at line 438). - [ ] **Step 2: Add the rendering block after the comments section** In `src/tui/widgets.rs`, find the `if !issue.comments.is_empty() { ... }` block (around line 438–462) and add a parallel block immediately after it, before `Text::from(lines)`: ```rust if !issue.linked_commits.is_empty() { lines.push(Line::raw("")); lines.push(Line::styled( "--- Linked Commits ---", Style::default() .fg(Color::Magenta) .add_modifier(Modifier::BOLD), )); for lc in &issue.linked_commits { let short_sha: String = lc.commit.chars().take(7).collect(); let (subject, commit_author) = git2::Oid::from_str(&lc.commit) .ok() .and_then(|oid| repo.find_commit(oid).ok()) .map(|commit| { let subject = commit.summary().unwrap_or("").to_string(); let author = commit.author().name().unwrap_or("unknown").to_string(); (subject, author) }) .unwrap_or_else(|| (String::new(), String::new())); let line_text = if commit_author.is_empty() { format!( "· linked {} (commit {} not in local repo) (linked by {}, {})", short_sha, short_sha, lc.event_author.name, lc.event_timestamp ) } else { format!( "· linked {} \"{}\" by {} (linked by {}, {})", short_sha, subject, commit_author, lc.event_author.name, lc.event_timestamp ) }; lines.push(Line::raw(line_text)); } } ``` The `repo` parameter is already in scope — check the enclosing function's signature. If `build_issue_detail` does *not* currently take `&Repository`, thread it through: look at how callers pass `patches` in — they also pass the repo, so the plumbing exists. If the function really doesn't have repo access, fall back to the "(commit not in local repo)" form unconditionally; the CLI renderer is the authoritative one and the TUI can degrade. - [ ] **Step 3: Build and check** Run: `cargo build` Expected: clean build. If there's a missing `repo` parameter, the error message will point at the exact spot; thread the `&Repository` argument through from the caller (`src/tui/mod.rs` or `src/tui/state.rs`) to `build_issue_detail` and retry. - [ ] **Step 4: Run clippy** Run: `cargo clippy --all-targets -- -D warnings` Expected: no warnings. - [ ] **Step 5: Run the existing TUI smoke tests to confirm no regression** Run: `cargo test --test dashboard_test` Expected: PASS (if a dashboard test suite exists — skip this step if it doesn't). - [ ] **Step 6: Commit** ```bash git add src/tui/widgets.rs src/tui/mod.rs src/tui/state.rs git commit -m "Render linked commits section in TUI issue detail Mirrors the CLI renderer: --- Linked Commits --- section after comments, with short SHA + subject + commit author + linked-by metadata. Degrades gracefully when the commit isn't locally available." ``` (If `src/tui/mod.rs` or `src/tui/state.rs` weren't touched, drop them from the add — `git status` will show what to include.) --- ## Task 10: Final verification and cleanup **Files:** none - [ ] **Step 1: Run the full test suite** Run: `cargo test` Expected: PASS across all targets. - [ ] **Step 2: Run clippy with deny warnings** Run: `cargo clippy --all-targets -- -D warnings` Expected: no warnings. - [ ] **Step 3: Manual end-to-end smoke test** In a scratch directory (the worktree is fine): ```bash cd /tmp rm -rf commit-link-smoke && mkdir commit-link-smoke && cd commit-link-smoke git init -b main git config user.name "Smoke" git config user.email "smoke@example.com" git commit --allow-empty -m "root" # Use the locally-built binary. BIN=/home/xanderle/dev/git-collab/target/debug/git-collab $BIN issue open -t "smoke test issue" ID=$($BIN issue list | head -1 | awk '{print $1}') echo "Issue: $ID" git commit --allow-empty -m "Fix thing Issue: $ID" git init --bare /tmp/commit-link-smoke-bare git remote add origin /tmp/commit-link-smoke-bare git push -u origin main $BIN init $BIN sync $BIN issue show $ID ``` Expected in the output: a `--- Linked Commits ---` section with one line like `· linked <sha> "Fix thing" by Smoke (linked by Smoke, 2026-...)`. - [ ] **Step 4: Commit any final cleanup** If the smoke test reveals anything missing (e.g. a poorly-formatted line, a missing newline), fix it and commit: ```bash git add -p git commit -m "Polish commit-link renderer based on smoke test feedback" ``` Otherwise, this task is a no-op commit-wise. --- ## Self-Review Checking the plan against the spec `docs/superpowers/specs/2026-04-12-commit-issue-link-design.md`: **Spec coverage:** - ✅ `Action::IssueCommitLink` variant — Task 1. - ✅ `src/commit_link.rs` module with `parse_issue_trailers`, `collect_linked_shas`, `scan_and_link` — Tasks 3, 4, 5. - ✅ Strict trailer-block rule (every non-empty line must be trailer-shaped) — Task 3 parser + test `parser_mixed_final_paragraph_rejects_all`. - ✅ Single-token value rule (`(\S+)` equivalent via `split_whitespace().count() != 1`) — Task 3 parser + test `parser_rejects_value_with_trailing_garbage`. - ✅ Case-insensitive key match — Task 3 test `parser_case_variants`. - ✅ Sync insertion point (after `reconcile_refs("patches")`, before `collect_push_refs`) — Task 6. - ✅ Revwalk of all `refs/heads/*` with per-sync `HashSet<Oid>` dedup — Task 5 (`scan_and_link`). - ✅ Per-issue linked-SHA cache keyed by resolved ref name — Task 5 (`link_cache: HashMap<String, Option<HashSet<String>>>`). - ✅ Archived-issue skip with warning — Task 5 (`ARCHIVED_ISSUE_PREFIX` check) + integration test in Task 7. - ✅ Detached HEAD / no local branches silent no-op — Task 5 (`seeded_any` check) + integration test in Task 7. - ✅ Cross-machine duplication handled at render time via first-seen dedup — Task 2 (`IssueState::from_ref_uncached` arm). - ✅ Renderer shows both commit author and event author — Tasks 8 (CLI) and 9 (TUI). - ✅ Renderer fallback when commit is not locally available — Tasks 8 and 9 (fallback branches). - ✅ "Never breaks sync" error handling philosophy — Task 5 (`eprintln!` warnings + `continue`). - ✅ Per-issue cache poisoning within a single sync — Task 5 (`Option<HashSet<...>>` where `None` is poisoned, `Some(_)` is live). - ✅ Unit tests for parser (all spec fixtures) — Task 3. - ✅ Integration tests: happy path, idempotency, multi-branch, multi-issue, unknown/ambiguous/archived, detached HEAD, remote-originated dedup, CLI rendering, render-time dedup — Tasks 2, 5, 6, 7, 8. **Placeholder scan:** - No "TBD", "TODO", "fill in later" in any task. - Every code block contains the actual code to write. - Every test has real assertions. - The TUI task has one conditional ("if `repo` isn't in scope, thread it through") — that's an *explicit* decision branch with a concrete fallback, not a placeholder. Acceptable. **Type/name consistency:** - `LinkedCommit { commit, event_author, event_timestamp }` used consistently across Task 2 struct definition, Task 2 materializer, Task 8 CLI renderer, Task 9 TUI renderer. - `scan_and_link(repo, author, sk) -> Result<usize, Error>` used consistently in Task 5 impl and Tasks 5, 6, 7 tests. - `collect_linked_shas(repo, issue_ref) -> Result<HashSet<String>, Error>` consistent Task 4 impl + Task 5 caller. - `parse_issue_trailers(message) -> Vec<String>` consistent Task 3 impl + Task 3 tests + Task 5 caller. - `Action::IssueCommitLink { commit }` — field name `commit` used in Task 1 variant, Task 2 materializer, Task 4 helper, Task 5 orchestrator, Task 2/5 test helpers. **Note on one simplification** introduced by the plan vs. the spec's wording: The spec says "each time a given issue is matched in a sync, we walk its DAG once to build the set, cache it". The plan implements this literally via `HashMap<String, Option<HashSet<String>>>` in `scan_and_link`, where `Option::None` represents a poisoned (read-failed) issue. The spec wording "poisoned sentinel" is preserved. The spec calls the renderer output an "issue-timeline line"; the actual code has no timeline, just separate `--- Comments ---` / `--- Linked Patches ---` sections. The plan adapts this to a new `--- Linked Commits ---` section matching the existing sectioned layout. This is a faithful translation of the spec's intent into the codebase's actual UI structure. --- ## Execution Handoff Plan complete and saved to `docs/superpowers/plans/2026-04-12-commit-issue-link.md`. Two execution options: **1. Subagent-Driven (recommended)** — I dispatch a fresh subagent per task, review between tasks, fast iteration. **2. Inline Execution** — Execute tasks in this session using executing-plans, batch execution with checkpoints. Which approach?