diff --git a/crates/signed_core/src/annotations.rs b/crates/signed_core/src/annotations.rs index e45365d..0a49935 100644 --- a/crates/signed_core/src/annotations.rs +++ b/crates/signed_core/src/annotations.rs @@ -5,244 +5,3 @@ use nostr::prelude::*; /// A markdown note attached to an issue, patch or PR by its author or a maintainer, /// not part of the NIP-34 draft, read support for interop. pub const COVER_NOTE_KIND: Kind = Kind::Custom(1624); - -/// Whether a kind-1985 label event is a valid annotation of `root`. -/// -/// The event references the root with a lowercase `e` tag, -/// its author must be the root author or a maintainer. -fn label_targets_root(event: &Event, root: &Event, maintainers: &[PublicKey]) -> bool { - if event.kind != Kind::Label { - return false; - } - if event.pubkey != root.pubkey && !maintainers.contains(&event.pubkey) { - return false; - } - let root_id = root.id.to_hex(); - event - .tags - .iter() - .any(|tag| tag.kind() == "e" && tag.content().is_some_and(|content| content == root_id)) -} - -/// Whether a kind-1985 label event declares the `#t` namespace, -/// it must also carry at least one `["l", "", "#t"]` label. -fn has_hashtag_labels(event: &Event) -> bool { - event.tags.iter().any(|tag| tag.as_slice() == ["L", "#t"]) - && event.tags.iter().any(|tag| { - let slice = tag.as_slice(); - slice.len() >= 3 && slice[0] == "l" && slice[2] == "#t" && !slice[1].is_empty() - }) -} - -/// Effective hashtag labels of `root`, -/// the `t` tags on the event itself, self-reported by its author, -/// authorized NIP-32 kind-1985 events in the `#t` namespace add more. -/// -/// Labels are additive, so all valid label events contribute, -/// there is no latest-wins semantics. -pub fn labels(root: &Event, label_events: &[Event], maintainers: &[PublicKey]) -> Vec { - let mut labels: Vec = root - .tags - .hashtags() - .map(|hashtag| hashtag.to_string()) - .collect(); - - for event in label_events { - if !label_targets_root(event, root, maintainers) || !has_hashtag_labels(event) { - continue; - } - for tag in event.tags.iter() { - let slice = tag.as_slice(); - if slice.len() >= 3 && slice[0] == "l" && slice[2] == "#t" && !slice[1].is_empty() { - let label = &slice[1]; - if !labels.contains(label) { - labels.push(label.clone()); - } - } - } - } - - labels -} - -/// Subject or title override of `root` from authorized kind-1985 label events, -/// only label events in the `#subject` namespace count. -/// -/// Returns `None` when no valid override exists. -pub fn subject_override( - root: &Event, - label_events: &[Event], - maintainers: &[PublicKey], -) -> Option { - label_events - .iter() - .filter(|event| label_targets_root(event, root, maintainers)) - .filter(|event| { - event - .tags - .iter() - .any(|tag| tag.as_slice() == ["L", "#subject"]) - && event.tags.iter().any(|tag| { - let slice = tag.as_slice(); - slice.len() >= 3 - && slice[0] == "l" - && slice[2] == "#subject" - && !slice[1].is_empty() - }) - }) - .max_by(|a, b| { - a.created_at - .cmp(&b.created_at) - .then_with(|| a.id.to_string().cmp(&b.id.to_string())) - }) - .and_then(|event| { - event.tags.iter().find_map(|tag| { - let slice = tag.as_slice(); - (slice.len() >= 3 - && slice[0] == "l" - && slice[2] == "#subject" - && !slice[1].is_empty()) - .then(|| slice[1].clone()) - }) - }) -} - -/// Effective hashtag labels and subject override of `root` in one pass, -/// mirrors ngit's `get_labels_and_subject`. -pub fn labels_and_subject( - root: &Event, - label_events: &[Event], - maintainers: &[PublicKey], -) -> (Vec, Option) { - ( - labels(root, label_events, maintainers), - subject_override(root, label_events, maintainers), - ) -} - -/// Effective cover note of `root`. -/// -/// Returns `None` when no valid cover note exists. -pub fn cover_note<'a>( - root: &Event, - cover_notes: &'a [Event], - maintainers: &[PublicKey], -) -> Option<&'a Event> { - let root_id = root.id.to_hex(); - - cover_notes - .iter() - .filter(|event| { - event.kind == COVER_NOTE_KIND - && (event.pubkey == root.pubkey || maintainers.contains(&event.pubkey)) - && event.tags.iter().any(|tag| { - tag.kind() == "e" && tag.content().is_some_and(|content| content == root_id) - }) - }) - .max_by(|a, b| { - a.created_at - .cmp(&b.created_at) - .then_with(|| a.id.to_string().cmp(&b.id.to_string())) - }) -} - -#[cfg(test)] -mod tests { - use super::*; - - fn keys_from_hex(hex: &str) -> Keys { - Keys::new(SecretKey::from_hex(hex).expect("valid secret key")) - } - - fn signed(author: &Keys, kind: Kind, tags: Vec, created_at: u64) -> Event { - EventBuilder::new(kind, "") - .tags(tags) - .custom_created_at(Timestamp::from(created_at)) - .finalize(author) - .expect("signed event") - } - - fn root_event() -> Event { - signed( - &keys_from_hex("0000000000000000000000000000000000000000000000000000000000000001"), - Kind::GitIssue, - vec![Tag::hashtag("bug")], - 100, - ) - } - - fn e_tag(event: &Event) -> Tag { - Tag::parse(["e", &event.id.to_hex()]).expect("valid e tag") - } - - #[test] - fn labels_take_inline_hashtags_and_external_label_events() { - let root = root_event(); - let maintainer = - keys_from_hex("0000000000000000000000000000000000000000000000000000000000000002"); - let labels_event = signed( - &maintainer, - Kind::Label, - vec![ - e_tag(&root), - Tag::parse(["L", "#t"]).expect("valid L tag"), - Tag::parse(["l", "help-wanted", "#t"]).expect("valid l tag"), - ], - 200, - ); - - let labels = labels(&root, &[labels_event], &[maintainer.public_key()]); - - assert_eq!(labels, vec!["bug", "help-wanted"]); - } - - #[test] - fn subject_override_latest_authorized_event_wins() { - let root = root_event(); - let maintainer = - keys_from_hex("0000000000000000000000000000000000000000000000000000000000000002"); - let older = signed( - &maintainer, - Kind::Label, - vec![ - e_tag(&root), - Tag::parse(["L", "#subject"]).expect("valid L tag"), - Tag::parse(["l", "Old title", "#subject"]).expect("valid l tag"), - ], - 200, - ); - let newer = signed( - &maintainer, - Kind::Label, - vec![ - e_tag(&root), - Tag::parse(["L", "#subject"]).expect("valid L tag"), - Tag::parse(["l", "New title", "#subject"]).expect("valid l tag"), - ], - 300, - ); - - assert_eq!( - subject_override(&root, &[newer, older], &[maintainer.public_key()]), - Some("New title".to_owned()) - ); - } - - #[test] - fn cover_note_latest_authorized_event_wins() { - let root = root_event(); - let maintainer = - keys_from_hex("0000000000000000000000000000000000000000000000000000000000000002"); - let stranger = - keys_from_hex("0000000000000000000000000000000000000000000000000000000000000003"); - let older = signed(&maintainer, COVER_NOTE_KIND, vec![e_tag(&root)], 200); - let newer = signed(&maintainer, COVER_NOTE_KIND, vec![e_tag(&root)], 300); - let unauthorized = signed(&stranger, COVER_NOTE_KIND, vec![e_tag(&root)], 400); - - let newer_id = newer.id; - let events = [older, unauthorized, newer]; - let maintainers = [maintainer.public_key()]; - let note = cover_note(&root, &events, &maintainers); - assert_eq!(note.map(|event| event.id), Some(newer_id)); - } -} diff --git a/crates/signed_core/src/clone_url.rs b/crates/signed_core/src/clone_url.rs deleted file mode 100644 index b66bf66..0000000 --- a/crates/signed_core/src/clone_url.rs +++ /dev/null @@ -1,87 +0,0 @@ -use nostr::prelude::*; - -use crate::RepoAddr; - -/// Target of a `nostr://` clone URL, as defined by NIP-34. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum CloneTarget { - /// `nostr://` encodes a direct repository address. - Addr(RepoAddr), - /// `nostr:///[relay-hint/]` - UserRepo { - /// `npub1...` or a NIP-05 identifier. - user: String, - relay_hint: Option, - /// `d` tag identifier of the repository. - identifier: String, - }, -} - -/// Parse a `nostr://` clone URL. Returns `None` for other URL schemes. -pub fn parse_clone_url(url: &str) -> Option { - let rest = url.strip_prefix("nostr://")?; - let mut parts = rest.split('/'); - - let first = parts.next()?; - let second = parts.next()?; - let third = parts.next(); - - if first.starts_with("naddr1") { - let coordinate = Nip19Coordinate::from_bech32(first).ok()?; - return Some(CloneTarget::Addr(coordinate.coordinate)); - } - - let (relay_hint, identifier) = match third { - Some(id) => ( - RelayUrl::parse(&percent_decode(second)).ok(), - percent_decode(id), - ), - None => (None, percent_decode(second)), - }; - - Some(CloneTarget::UserRepo { - user: first.to_owned(), - relay_hint, - identifier, - }) -} - -fn percent_decode(input: &str) -> String { - let bytes = input.as_bytes(); - let mut out = Vec::with_capacity(bytes.len()); - let mut i = 0; - while i < bytes.len() { - if bytes[i] == b'%' && i + 2 < bytes.len() { - let hex = &input[i + 1..i + 3]; - if let Ok(v) = u8::from_str_radix(hex, 16) { - out.push(v); - i += 3; - continue; - } - } - out.push(bytes[i]); - i += 1; - } - String::from_utf8_lossy(&out).into_owned() -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn decodes_percent_encoded_parts() { - let target = parse_clone_url( - "nostr://danconwaydev.com/ws%3A%2F%2Flocalhost%3A7334/my-local-only-repo", - ) - .unwrap(); - assert_eq!( - target, - CloneTarget::UserRepo { - user: "danconwaydev.com".to_owned(), - relay_hint: RelayUrl::parse("ws://localhost:7334").ok(), - identifier: "my-local-only-repo".to_owned(), - } - ); - } -} diff --git a/crates/signed_core/src/filters.rs b/crates/signed_core/src/filters.rs index 04a6f18..829a0ab 100644 --- a/crates/signed_core/src/filters.rs +++ b/crates/signed_core/src/filters.rs @@ -91,18 +91,6 @@ pub fn statuses_for(roots: impl IntoIterator) -> Filter { .events(roots) } -/// Cover notes and NIP-32 label events referencing any of the given root events. -/// These are kinds 1624 and 1985, matched via the `#e` tag. -/// -/// Because they carry no repository `a` tag, they are fetched by root like comments. -/// -/// Batched, like [`statuses_for`]. -pub fn annotations_for(roots: impl IntoIterator) -> Filter { - Filter::new() - .kinds([crate::COVER_NOTE_KIND, Kind::Label]) - .events(roots) -} - /// A user's grasp list, kind `10317`. pub fn grasp_list(public_key: PublicKey) -> Filter { Filter::new() diff --git a/crates/signed_core/src/lib.rs b/crates/signed_core/src/lib.rs index e7ecb69..2bc3ca6 100644 --- a/crates/signed_core/src/lib.rs +++ b/crates/signed_core/src/lib.rs @@ -1,6 +1,5 @@ pub mod addr; pub mod annotations; -pub mod clone_url; pub mod deletions; pub mod filters; pub mod inbox; @@ -9,8 +8,7 @@ pub mod state; pub mod status; pub use addr::{RepoAddr, identifier_from_name, repo_addr}; -pub use annotations::{COVER_NOTE_KIND, cover_note, labels_and_subject, subject_override}; -pub use clone_url::{CloneTarget, parse_clone_url}; +pub use annotations::COVER_NOTE_KIND; pub use deletions::Deletions; pub use filters::{ NOTIFICATION_KINDS, authored_activity, is_git_activity, notification_comments, notifications, diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 8873381..6d3cab4 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -1086,10 +1086,6 @@ impl Backend { self.signer.clone() } - pub fn pushing_repos(&self) -> Entity> { - self.pushing_repos.clone() - } - pub fn inbox(&self) -> Entity { self.inbox.clone() } @@ -1102,10 +1098,6 @@ impl Backend { self.passphrase_required } - pub fn emit_error(&mut self, message: impl Into, cx: &mut Context) { - cx.emit(BackendEvent::error(message)); - } - fn sync_inbox(&mut self, cx: &mut Context) { let client = self.client.clone(); let me = self.current_user; diff --git a/crates/signed_state/src/profile.rs b/crates/signed_state/src/profile.rs index d9ae4c4..2e25ac5 100644 --- a/crates/signed_state/src/profile.rs +++ b/crates/signed_state/src/profile.rs @@ -50,7 +50,7 @@ impl Profile { return SharedString::from(name.trim().to_owned()); } - SharedString::from(shorten_pubkey(self.public_key, 4)) + SharedString::from(shorten_pubkey(self.public_key)) } pub fn picture(&self) -> Option { diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 4770353..395f1e2 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -56,10 +56,6 @@ pub struct RepoStore { /// Computed with [`Self::status_by_root`] on every refresh. open_issue_count: usize, open_pr_count: usize, - /// Incremented on every applied refresh. - /// - /// Views key their derived-data caches to it instead of recomputing on every render. - version: u64, pub last_error: Option, /// Non-fatal warning of the last action, if any. /// @@ -125,7 +121,6 @@ impl RepoStore { status_by_root: HashMap::new(), open_issue_count: 0, open_pr_count: 0, - version: 0, last_error: None, last_warning: None, last_push_warning: None, @@ -153,7 +148,6 @@ impl RepoStore { status_by_root: HashMap::new(), open_issue_count: 0, open_pr_count: 0, - version: 0, last_error: None, last_warning: None, last_push_warning: None, @@ -401,9 +395,6 @@ impl RepoStore { } } - // Cover notes, 1624, and label events, 1985, carry no `a` tag. - // Query them per root like comments and statuses. - // The events are only stored for interop and nothing displays them. sort_newest_first(&mut issues); sort_newest_first(&mut patches); sort_newest_first(&mut pull_requests); @@ -522,7 +513,6 @@ impl RepoStore { this.open_issue_count = open_issue_count; this.open_pr_count = open_pr_count; this.loaded = true; - this.version = this.version.wrapping_add(1); // Comments and statuses without an `a` tag. // None are addressed to the repository. @@ -580,10 +570,6 @@ impl RepoStore { status_of(&self.status_by_root, root) } - pub fn version(&self) -> u64 { - self.version - } - /// Number of open issues. /// /// Issues whose resolved status is [`RepoStatus::Open`]. diff --git a/crates/signed_ui/src/dropdown_button.rs b/crates/signed_ui/src/dropdown_button.rs index e14d37c..1a257b1 100644 --- a/crates/signed_ui/src/dropdown_button.rs +++ b/crates/signed_ui/src/dropdown_button.rs @@ -8,22 +8,18 @@ use gpui_component::menu::PopupMenu; use gpui_component::{ActiveTheme, Icon, IconName, Sizable, h_flex}; /// A split dropdown button built on `gpui_base::Popover`. -/// An action element with a separate caret trigger that opens a [`PopupMenu`]. -/// The action and the caret are caller-supplied elements, so the look stays in the app. -/// This component only owns the popover wiring. +/// An action element next to a caret that opens a [`PopupMenu`]. #[derive(IntoElement)] pub struct DropdownButton { id: ElementId, style: StyleRefinement, anchor: Anchor, action: Option, - caret: Option, menu: Option, } type MenuBuilder = Box) -> PopupMenu + 'static>; -type CaretBuilder = Box AnyElement>; impl DropdownButton { pub fn new(id: impl Into) -> Self { @@ -32,7 +28,6 @@ impl DropdownButton { style: StyleRefinement::default(), anchor: Anchor::TopRight, action: None, - caret: None, menu: None, } } @@ -54,14 +49,6 @@ impl DropdownButton { self.menu = Some(Box::new(builder)); self } - - /// Which corner of the caret the menu anchors to. - /// Defaults to [`Anchor::TopRight`], lining the menu's right edge up with the caret's. - #[allow(dead_code)] // API knob, current call sites use the default anchor. - pub fn anchor(mut self, anchor: impl Into) -> Self { - self.anchor = anchor.into(); - self - } } impl Styled for DropdownButton { @@ -91,28 +78,24 @@ impl RenderOnce for DropdownButton { let menu_state = window.use_keyed_state(popover_id.clone(), cx, |_, _| DropdownMenuState::default()); - let caret = self.caret.unwrap_or_else(|| { - let id = popover_id.clone(); - Box::new(move |is_open, _, cx| { - let caret = default_caret(id.clone(), cx); - let selected = caret.is_selected(); - caret.selected(selected || is_open).into_any_element() - }) - }); - h_flex() .id(self.id) .refine_style(&self.style) .gap_0p5() .when_some(self.action, |this, action| this.child(action)) .when_some(self.menu, |this, builder| { + let caret_id = popover_id.clone(); this.child( Popover::new(popover_id) .anchor(anchor) // The menu dismisses itself on outside click or Escape. // The subscription below closes the popover along with it. .overlay_closable(false) - .trigger_with(caret) + .trigger_with(move |is_open, _, cx| { + let caret = default_caret(caret_id.clone(), cx); + let selected = caret.is_selected(); + caret.selected(selected || is_open).into_any_element() + }) .content( move |_, window, cx| match menu_state.read(cx).menu.clone() { Some(menu) => menu, diff --git a/crates/utils/src/pubkey.rs b/crates/utils/src/pubkey.rs index 3b2d588..4e0d19f 100644 --- a/crates/utils/src/pubkey.rs +++ b/crates/utils/src/pubkey.rs @@ -1,7 +1,14 @@ use nostr::prelude::*; /// Shorten a [`PublicKey`] to `npub1abc...wxyz` form. -pub fn shorten_pubkey(public_key: PublicKey, len: usize) -> String { +pub fn shorten_pubkey(public_key: PublicKey) -> String { + const HEAD_CHARS: usize = 9; + const TAIL_CHARS: usize = 4; + let npub = public_key.to_bech32().unwrap(); - format!("{}...{}", &npub[..(len + 5)], &npub[npub.len() - len..]) + format!( + "{}...{}", + &npub[..HEAD_CHARS], + &npub[npub.len() - TAIL_CHARS..] + ) } diff --git a/docs/TODO.md b/docs/TODO.md new file mode 100644 index 0000000..a12edc4 --- /dev/null +++ b/docs/TODO.md @@ -0,0 +1,19 @@ +# TODO + +Deferred from `docs/over-engineering-cleanup-plan.md`. + +## `BackendEvent::SyncProgress` + +File: `crates/signed_state/src/backend.rs` + +The variant has no subscriber. Remove the variant, the `sync_progress` field and +its accessor, and the `progress_task` in `sync_bootstrap`. Keep the terminal +`Synced` emission. + +## `login` / `logout` family + +File: `crates/signed_state/src/backend.rs` + +No UI path calls these. `import_dialog::open` is an empty stub. Decide whether to +delete the family (`login`, `login_with_new_identity`, `login_with_nsec`, +`login_with_bunker`, `logout`) or wire the stub to `Backend::login`. diff --git a/docs/over-engineering-cleanup-plan.md b/docs/over-engineering-cleanup-plan.md new file mode 100644 index 0000000..2dcb908 --- /dev/null +++ b/docs/over-engineering-cleanup-plan.md @@ -0,0 +1,283 @@ +# Over-engineering cleanup plan + +## Goal + +Remove code that exists but cannot be reached, and machinery that guards states no +caller can produce. Findings came from a four-way read-only audit of +`signed_state`, `workspace/views`, `signed_git`/`signed_core`, and +`dock`/`signed_ui`/misc, cross-checked against the pinned dependency sources in +`~/.cargo/git/checkouts/`. + +The main claim in each task below was verified by grepping callers, not by +reading the definition alone. Items that were only reported by the audit and not +independently reproduced are in Phase 4 and must be verified before deletion. + +## Rules for every step + +- Line numbers are from the current working tree and will drift. Re-grep before + editing; do not trust a number from this file after other tasks land. +- Delete code, do not comment it out, do not add `#[allow(dead_code)]`. +- If a symbol looks dead but is part of a public API or a feature that is only + not wired yet, stop and ask. +- Do not reintroduce the `views/repo` generation counters that were removed in + this working tree. +- Keep comments out. Remove any comment that describes the code being deleted. + +## Verification commands + +Always pass `--offline`; a plain `cargo` invocation re-resolves and fails in the +sandbox. + +``` +cargo fmt -p 2>/dev/null +cargo check --offline --workspace --all-targets +cargo clippy --offline --workspace --all-targets +cargo test --offline --workspace +``` + +`cargo fmt -- --check` prints unrelated "unstable features" noise on stable. +Filter with `grep -E "^Diff in"`. Never hand-reformat; rustfmt is authoritative. + +--- + +## Phase 1 - delete dead code + +No behavior change. Each task is independent; commit per crate. + +### 1.1 `RepoStore::version` + +File: `crates/signed_state/src/repo.rs` + +- [x] Remove field `version: u64` and its doc comment (the claim that views key + caches to it is false). +- [x] Remove initializers `version: 0` in both constructors. +- [x] Remove the bump `this.version = this.version.wrapping_add(1);`. +- [x] Remove `pub fn version(&self) -> u64`. + +Evidence: `grep -rn "\.version()" crates` has no call sites; the only read is the +accessor itself. + +Acceptance: `grep -rn "version" crates/signed_state/src/repo.rs` shows only +unrelated uses (none of the four removed sites). + +### 1.2 `clone_url` module + +Files: `crates/signed_core/src/clone_url.rs`, `crates/signed_core/src/lib.rs` + +- [x] Delete `clone_url.rs`. +- [x] Remove `mod clone_url;` and the `pub use clone_url::{CloneTarget, parse_clone_url};` + re-export. + +Evidence: only definition, its own test, and the re-export reference these. It +also reimplements percent-decoding. + +### 1.3 NIP-32 labels / cover-note helpers + +Files: `crates/signed_core/src/annotations.rs`, `crates/signed_core/src/filters.rs`, +`crates/signed_core/src/lib.rs` + +- [x] Verify each of `labels`, `subject_override`, `labels_and_subject`, + `cover_note`, `COVER_NOTE_KIND`, `annotations_for` for references outside + this crate. +- [x] Delete the ones with no production caller and drop them from the `lib.rs` + re-exports. +- [x] Keep anything still needed (for example `cover_note` / + `COVER_NOTE_KIND` may be used by the inbox view). + +Evidence: the UI uses `tags.hashtags()` directly in `views/discussion.rs`, not +these helpers. + +### 1.4 `Backend::emit_error` + +File: `crates/signed_state/src/backend.rs` + +- [x] Delete the method. No callers. + +### 1.5 `Backend::pushing_repos()` accessor + +File: `crates/signed_state/src/backend.rs` + +- [x] Delete the getter. The `Entity>` is used internally; only + the accessor is unused. +- [ ] Optional follow-up (separate task): nothing observes that entity, so it + could be a plain `HashSet` field. Defer; it is a refactor, not a deletion. + +### 1.6 `DropdownButton` speculative knobs + +File: `crates/signed_ui/src/dropdown_button.rs` + +- [x] Remove the `caret: Option` field and the `CaretBuilder` type + alias; it is never set, so the `unwrap_or_else` default always runs. + Inline the default caret. +- [x] Remove the `anchor()` builder method (never called; it is already marked + `#[allow(dead_code)]`). Keep the `anchor` field, which is set in the + constructor and used when rendering. + +### 1.7 `utils::shorten_pubkey` + +File: `crates/utils/src/pubkey.rs` + +- [x] Drop the `len` parameter; its only call site passes `4`. +- [x] Rename to a fixed-width helper if that reads better, or leave the name. + +It duplicates `signed_ui::middle_truncate` conceptually, but `utils` has no gpui +dependency, so do not move `middle_truncate`; just remove the speculative +parameter. + +### 1.8 Comment artifacts + +- [x] `crates/signed_state/src/repo.rs` - delete the comment that describes + querying cover notes and labels per root; no such query exists. +- [x] Remove any comment left dangling by the tasks above. + +--- + +## Phase 2 - remove guards that cannot fire + +Each changes behavior on paper but not in practice. Smoke test after each. + +### 2.1 `SignedDockSkin::render_dock` early return and frame duplication + +File: `crates/dock/src/dock_area.rs` + +Base (`gpui_base::dock`) computes `dock_extent`, returns before calling the +renderer when the extent is `px(0.)`, and wraps the renderer's output in +`dock_frame`. `dock_extent` is `px(0.)` exactly when `!open && !is_bottom`, which +is precisely the condition of the early return here. + +- [ ] Delete the `if !open && !placement.is_bottom() { return div(); }` guard. +- [ ] Stop re-applying `.flex().flex_none().relative().overflow_hidden()` and the + per-placement width/height; base already applies them. +- [ ] Keep the closed-bottom strip height override, but confirm against base's + `CLOSED_BOTTOM_STRIP` that the intended height is `TAB_BAR_HEIGHT`. +- [ ] Smoke test: open and close left, bottom, and right docks; check widths, + the bottom strip height, and resize handles. + +### 2.2 `push_staged_to_grasps` empty-refs guard + +File: `crates/signed_state/src/backend.rs` + +- [ ] Delete the `if refs.is_empty() { return outcome; }` guard. + +All three call sites pass a non-empty `refs`: one passes a literal one-element +vec, one is inside `if !refs.is_empty()`, one is the `else` of that check. + +### 2.3 `InboxView` per-view debounce + +File: `crates/workspace/src/views/inbox.rs` + +The backend pump already coalesces relay bursts into one `NostrUpdate`, and +`query_inbox` reads only the local database. + +- [ ] Delete the `REFRESH_DEBOUNCE` constant. +- [ ] In `refresh`, drop the spawned timer; call `run_refresh` directly after + `refresh.request()` returns `Schedule`, matching `RepoStore::refresh`. +- [ ] Keep `RefreshGate` for fold/overlap. +- [ ] Smoke test: inbox updates live as relay events land, with no added delay. + +### 2.4 `DockPlacement::Center` arms in `dock_toggle_button` (optional) + +File: `crates/dock/src/tab_panel.rs` + +- [ ] The only call sites pass `Left`, `Bottom`, `Right`. Collapse the `Center` + arms to `unreachable!()` or restructure so the match is exhaustive without + a dead branch. + +Low value; skip if it makes the match less readable. + +--- + +## Phase 3 - consolidate duplication (needs a decision) + +Verify the duplication before extracting; each could be intentional. + +### 3.1 `PullRequestsView` and `IssuesView` + +Files: `crates/workspace/src/views/pull_requests/mod.rs`, +`crates/workspace/src/views/issues/mod.rs` + +- [ ] Confirm the filter enum, visible-index rebuild, counts tuple, and + virtual-list resize are the same shape. +- [ ] If so, extract one small helper for the filtered index + counts + notify + decision and use it in both. + +### 3.2 Relay URL normalize/display + +Files: `crates/workspace/src/views/sidebar/settings_dialog.rs`, +`crates/workspace/src/views/sidebar/grasp_servers.rs` + +- [ ] Confirm both pairs do prepend-scheme, parse, dedupe, and host-display. +- [ ] Extract one normalize helper and one display helper. Decide the crate + (check whether `signed_ui` may depend on `nostr`). + +### 3.3 `crates/dock` vs the pinned `gpui_component` dock renderer + +Files: `crates/dock/src/*` vs the pinned rev's `crates/ui/src/dock/*` + +- [ ] Spike only: pick one part (`SignedTabGroupSkin` or `SignedTilesSkin`) and + determine whether it can delegate to the upstream `DockSkin` trait + implementation and keep only the Signed deltas (window controls in the tab + bar, plain-sidebar detection, prev/next, i18n). +- [ ] Report effort and risk before doing any replacement. Do not start a + rewrite of this crate in this cleanup. + +--- + +## Phase 4 - triage flagged items + +Verify each, then delete or dismiss. These were reported by the audit but not +independently reproduced. + +- [ ] `signed_git/src/worktree.rs` - manual `refs/heads/{rev}` fallback; check + whether gix's ref DWIM already covers it. +- [ ] `signed_git/src/repo.rs` - `refs_with_prefix`; check + `repo.references()?.prefixed(prefix)`. +- [ ] `signed_git/src/patch.rs` - the mbox envelope is scanned twice; check + whether `patch_commits` can consume `split_patch_series` output, and + whether the hard-coded 40-hex checks should use `gix::ObjectId::from_hex`. +- [ ] `assets/src/lib.rs` - `themes` handles a `Cow::Owned` case the build + features cannot produce. +- [ ] `signed_state/src/backend.rs` - `GraspServerResult::git_url` populated but + never read. +- [ ] `signed_nostr/src/signer.rs` - `UniversalSignerError` vs + `nostr::Error::other`. Keep the `InnerSigner` erasure shim; only the error + wrapper is replaceable. +- [ ] `signed_git/src/history.rs` - `last_commit` referenced only from tests. +- [ ] `signed_git/src/repo.rs` - `init_repository` / `root_commit` 40-length + guards on an `ObjectId` string. +- [ ] `signed_core/src/model.rs` - `Upstream.relay_hint` parsed but unused in + production. +- [ ] `signed_core/src/inbox.rs` - `root_kind` duplicates `root_event`; confirm + before removing, it is read by the inbox view. + +--- + +## Do not touch + +- The two `pull_requests` generation counters (`load_generation` in + `detail.rs`, `compare_generation` in `new.rs`). Both were verified reachable. +- Tasks stored in a `Vec>` for lifetime cancellation. This is + intentional. +- `RefreshGate` on `CheckoutsStore`; its timer-driven debounce is load-bearing. +- The `dev`-time `init_dialog.rs` `.detach()`; the task owns a window-scoped + dialog and has no owning struct. + +## Acceptance criteria + +- Every removed symbol returns empty for a repo-wide grep. +- No new `#[allow(dead_code)]`. +- `cargo fmt -- --check` diff-free, `cargo check --offline --workspace + --all-targets` clean, `cargo clippy --offline --workspace --all-targets` + clean, `cargo test --offline --workspace` green. +- Manual smoke: open and close docks, watch the inbox update live, open a repo + and switch branches, publish a repo. + +## Suggested commit sequence + +1. `signed_state`: 1.1, 1.4, 1.5 (dead code), plus 1.8 comments in the same files. +2. `signed_core`: 1.2, 1.3 (dead modules). +3. `signed_ui` + `utils`: 1.6, 1.7. +4. `dock`: 2.1, 2.4. +5. `signed_state`: 2.2. +6. `workspace`: 2.3. +7. Phases 3 and 4 as separate, individually reviewed changes.