docs/push-refactor.md

@@ -1,164 +1,165 @@
-# Push refactor plan
+# Push refactor
 
-Refactor the grasp push pipeline into `SyncState` methods with step-by-step
-progress the UI can render while a push is in flight.
+Move the push pipeline onto `SyncState` with step-by-step progress, so the UI can render a live checklist instead of a single spinner.
 
-## Motivation
+## Problems
 
-- `push_staged_to_grasps` takes 9 parameters (needs `#[allow(clippy::too_many_arguments)]`)
-  and mixes three concerns per iteration: signing a Nostr event, relay
-  connection/staging, and git transport.
-- The flow is orchestrated in two places with different error handling:
-  `Backend::push_repo_from` and `announce_repository_and_push` in
-  `signed_state/src/backend.rs`.
-- The state event is rebuilt and re-signed per server inside the loop, forcing
-  `PushOutcome::keep_newest` to exist.
-- Feedback is binary: `RepoStore::pushing` plus a final `Result`. The loop runs
-  inside an opaque `background_spawn`, so the UI cannot show where a push is.
+- `push_staged_to_grasps` is a free function with 9 arguments; `refs`/`head` are just threaded through. Both call sites wrap it in another `background_spawn` with ~8 lines of cloning (`backend.rs` `push_repo_from`, `announce_repository_and_push`).
+- The state event is re-signed per relay inside the loop.
+- There is no progress surface: the whole flow is one opaque `Task<Result<PushOutcome>>`. The UI only has the `pushing` flag and the post-hoc `partial_warning`.
+- `RepoState` lives in `signed_core`, but its only consumers are in `signed_state`. `signed_git::RepoRefState` is structurally identical (`refs` + `head`).
 
-Moving `signed_core/src/state.rs` into `signed_state` is safe: `RepoState` is
-only used inside `signed_state` (`push.rs`, `repo.rs`, `sync_status.rs`), and
-`signed_state` already depends on `nostr` directly. `Kind::RepoState` references
-are the Nostr kind enum and are unaffected.
+## Constraints
+
+- Keep it simple; no edge-case handling, no over-engineering.
+- No new struct wrapping the Nostr client or signer; reference them.
+- No new global entity.
 
 ## Target shape
 
+- `signed_state/src/state.rs` (moved from core, `RepoState` renamed to `SyncState`): the data (`refs`, `head`, push `path`, `mode`), builders (`from_repo`, `with_mode`), event `build`/`parse`, and the whole push flow as methods on `impl SyncState`.
+- `signed_state/src/push.rs`: keeps `GraspServer`, `PushOutcome` and the grasp URL helpers. No flow logic.
+- `Backend`: thin wiring, emits `BackendEvent::PushProgress` per step.
+- `RepoStore`: owns the displayed step list (it already subscribes to `Backend`).
+- `RepoDetailView`: renders the checklist.
+
+## Step 1 — Move and rename
+
+Mechanical, no behavior change.
+
+- Move `crates/signed_core/src/state.rs` to `crates/signed_state/src/state.rs`, rename `RepoState` to `SyncState`. Tests move along.
+- `signed_core/src/lib.rs`: drop `pub mod state;` and the re-export.
+- `signed_state/src/lib.rs`: `mod state;` and `pub use state::SyncState;`.
+- Update importers in `signed_state`: `push.rs`, `repo.rs`, `sync_status.rs`. Nothing else uses `signed_core::RepoState`. `Kind::RepoState` (the nostr kind) is untouched.
+
+## Step 2 — Push flow under `impl SyncState`
+
+Replace `push_staged_to_grasps` with methods on the type that owns the refs:
+
 ```rust
-// signed_state/src/sync_state.rs
+#[derive(Debug, Clone, PartialEq, Eq, Default)]
+pub enum GitPushMode {
+    /// Mirror all branches and tags.
+    #[default]
+    All,
+    /// Push `refs/heads/main` only (repo creation).
+    Main,
+}
+
 pub struct SyncState {
     pub refs: Vec<(String, String)>,
     pub head: Option<String>,
+    /// Local repository to push from; `None` for states parsed from events.
+    pub path: Option<PathBuf>,
+    pub mode: GitPushMode,
 }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub enum PushStep {
-    ReadLocalState,
-    Announce,
-    StageStateEvent(RelayUrl),
-    PushGitObjects(RelayUrl),
-    PublishStateEvent,
+    AnnounceRepository,
+    StageStateEvent(RelayUrl), // publish state event to one grasp server
+    PushObjects(RelayUrl),     // git push to one grasp server
+    BroadcastStateEvent,       // fan the state event out to relays
 }
 
-pub enum StepStatus {
-    Running,
-    Done,
-    Failed(String),
-}
-
-pub struct PushProgress {
-    pub steps: Vec<(PushStep, StepStatus)>,
+#[derive(Debug, Clone)]
+pub enum PushStepStatus {
+    Running(PushStep),
+    Done(PushStep),
+    Failed(PushStep, String),
 }
 ```
 
-`PushProgress` is a GPUI `Entity` the caller creates and passes in; views
-observe it and re-render as steps update. Label mapping for the UI:
-
-- `ReadLocalState` -> "Read local refs"
-- `Announce` -> "Announce repository"
-- `StageStateEvent(relay)` -> "Publish state event to {relay}"
-- `PushGitObjects(relay)` -> "Push to grasp {relay}"
-- `PublishStateEvent` -> "Publish state event on relays"
-
 ```rust
 impl SyncState {
-    pub fn parse(event: &Event) -> Self;
-    pub fn build(id: &str, refs: &[(String, String)], head: Option<&str>) -> EventBuilder;
-    pub fn from_repo(repo: &Repo) -> Result<Self>;
-    pub async fn push_to_grasps(
+    /// Reads the refs and head of a local repository, ready to push.
+    pub fn from_repo(path: &Path) -> Result<Self> {
+        let repo = Repo::open(path)?;
+        let state = repo.ref_state()?;
+        Ok(Self {
+            refs: state.refs,
+            head: state.head,
+            path: Some(path.to_path_buf()),
+            mode: GitPushMode::default(),
+        })
+    }
+
+    pub fn with_mode(mut self, mode: GitPushMode) -> Self {
+        self.mode = mode;
+        self
+    }
+
+    /// State event for these refs; the caller signs it.
+    pub fn state_event(&self, repo_id: &str) -> EventBuilder {
+        Self::build(repo_id, &self.refs, self.head.as_deref())
+    }
+
+    // Per server: connect + send the pre-signed state event, then git push
+    // via cx.background_spawn (blocking gix work). Reports every transition
+    // through on_step. Ends with the relay fan-out broadcast. Errors once
+    // up front when self.path is None (state not built from a local repo).
+    pub(crate) async fn push_to_grasps<P>(
         &self,
-        plan: PushPlan,
-        progress: Entity<PushProgress>,
         cx: &mut AsyncApp,
-    ) -> PushOutcome;
-    async fn stage_on_grasp(...) -> Result<(), String>;  // connect + purgatory staging
-    async fn publish(...);                               // final broadcast
+        client: &Client,
+        event: Event,
+        announcement: &Announcement,
+        on_step: &mut P,
+    ) -> Result<PushOutcome, Error>
+    where
+        P: FnMut(PushStepStatus, &mut AsyncApp) -> Result<(), Error>,
+    { ... }
+
+    // announce_repository_and_push moved as-is: sign + broadcast the
+    // announcement (AnnounceRepository step), parse it into an
+    // `Announcement`, sign the state event, delegate to push_to_grasps,
+    // retract on total failure. This one keeps the signer.
+    pub(crate) async fn announce_and_push<S, P>(...) -> Result<Event, Error> { ... }
 }
 ```
 
-The key enabler: the per-server orchestration loop moves from
-`background_spawn` into the foreground `cx.spawn` task. Each blocking piece
-(`ref_state`, git push) stays on background threads; network awaits are fine on
-the foreground executor. Each step can then update `PushProgress` directly.
+### Why these arguments
 
-Flow:
+The struct carries the job description — what (`refs`, `head`), from where (`path`), how much (`mode`) — built with `SyncState::from_repo(path)?.with_mode(..)`. The method arguments are then only what varies per call or cannot live on the data: `cx`, `client`, the pre-signed `event`, `announcement`, `on_step`. Down from the 9-arg free function:
 
-```
-UI action
-  -> Backend push method
-    -> ReadLocalState            (background git)
-    -> Announce                  (sign + broadcast, create/publish flows only)
-    -> for each grasp server:
-         StageStateEvent(relay)  (sign once, send to relay)
-         PushGitObjects(relay)   (background git)
-    -> PublishStateEvent         (broadcast)
-  -> every step updates Entity<PushProgress> -> views render the checklist
-```
+- `refs`, `head`, `path`, `mode` come from `&self`. The `push: fn(&Path, &str, &str, &str) -> Result<(), Error>` callback becomes the `mode` field, resolved inside to `Repo::push_main` / `Repo::push_all`; the wildcard-refspec mirror semantics of `push_all` are preserved. States built by `SyncState::parse` (remote events) have no local repo, so `path` is `Option<PathBuf>` and `push_to_grasps` errors once up front when it is `None`.
+- `repo_id`, `owner`, `servers` collapse into `&Announcement` — its `id`, `owner` and `relays` are exactly those values, and both call sites already hold one. `push_repo_from` currently destructures the announcement into those three fields just to re-pass them. It stays an argument because states parsed from events carry no announcement.
+- The state event is signed by the caller via `state_event(..).finalize_async(&signer)`, so `push_to_grasps` takes a signed `Event` and needs no signer or `S` generic — it only ever sends events. The signer lives with the callers (`push_repo_from`, `announce_and_push`, which also signs the announcement and the retraction).
+- `client` stays as a reference per the constraints; `SyncState` remains plain data, so `SyncState::parse` keeps working for remote states that have no client or path.
+- `on_step` takes `&mut AsyncApp` so it can update an entity directly. That is what lets the whole flow run in the caller's foreground `cx.spawn` task and emit progress without a channel.
 
-## Steps
+### Progress mechanics
 
-### 1. Move and rename (mechanical)
+- Per-relay git push is backgrounded inside the method (`cx.background_spawn`), so the method itself runs in the caller's foreground task. Foreground nostr awaits are already precedent (`connect_grasp_relays`, `set_signer`).
+- `on_step` returns `Result` so a dead entity propagates instead of being swallowed.
+- The caller signs the state event up front instead of per relay; a signing failure aborts before any network work.
+- The final fan-out broadcast moves into `push_to_grasps`; both existing flows did exactly push, bail if zero accepted, then broadcast.
 
-- `git mv crates/signed_core/src/state.rs crates/signed_state/src/sync_state.rs`,
-  rename `RepoState` -> `SyncState`.
-- `signed_core/src/lib.rs`: drop `pub mod state;` and the re-export. No
-  deprecated alias; there are no users outside `signed_state`.
-- `signed_state/src/lib.rs`: add `mod sync_state;` and
-  `pub use sync_state::SyncState;`.
-- Update the three import sites: `push.rs`, `repo.rs`, `sync_status.rs`.
-- Tests in the file move along unchanged.
-- Validate: `cargo check -p signed_core -p signed_state && cargo test -p signed_state`.
+## Step 3 — Backend wiring
 
-### 2. Consolidate the push core onto `SyncState` (behavior-preserving)
+- Add `BackendEvent::PushProgress { addr: RepoAddr, status: PushStepStatus }`.
+- `push_repo_from`: background-spawn `SyncState::from_repo(&path)`, sign `state.state_event(&announcement.id)` with the signer, then call `push_to_grasps(cx, &client, event, &announcement, on_step)` in the same foreground task, `on_step` emitting the event via `this.update`. The nested `background_spawn` and its clone block disappear. Keep the `refs.is_empty()` early return at the call site.
+- `announce_repository_and_push` is deleted from `backend.rs`; its callers build a `SyncState` first (`SyncState::from_repo` for publish; repo creation builds the single-ref state literally with `GitPushMode::Main`), then call `announce_and_push(cx, client, signer, announcement, on_step)`.
+- `push.rs` keeps `GraspServer`, `PushOutcome` and the URL helpers.
 
-- Introduce `PushPlan { client, signer, repo_id, owner, servers, path, kind }`
-  to kill the 9-arg signature. Replace the `push` closure with
-  `enum PushKind { Main, All }` - the only two variants used across all call
-  sites (`push_main` for create, `push_all` otherwise).
-- Sign the state event once before the server loop instead of per iteration
-  (BIP-340 signing is deterministic; `created_at` is fixed at build time).
-  Delete `PushOutcome::keep_newest`. A signing failure now aborts the push up
-  front instead of "retrying" per server; key errors are not transient.
-- Split the loop body into named methods: `SyncState::stage_on_grasp` (connect
-  + staging match block) and the git-push arm. Move the final state-event
-  broadcast (duplicated in both call sites today) into `SyncState::publish`.
-- Fold `GraspServer`, `PushOutcome`, and the `grasp_*` URL helpers into
-  `sync_state.rs` and delete `push.rs`.
-- `announce_repository_and_push` shrinks to: connect -> sign/broadcast
-  announcement -> `state.push_to_grasps(plan, ...)` -> retraction on failure.
-- Validate: full `cargo test` for the signed crates; outcomes must be
-  identical (same events, same `PushOutcome`).
+## Step 4 — UI checklist
 
-### 3. Add step-by-step progress
+- `RepoStore`: add `push_steps: Vec<PushStepStatus>`; clear it in `run_push` start/end; handle `BackendEvent::PushProgress` in the existing `subscribe_backend` (match on `addr`, replace by `PushStep` equality, `cx.notify()` — no refresh).
+- `RepoDetailView`: while `pushing`, render the checklist where the spinner is today (header menu / push banner):
 
-- Add `PushStep`, `StepStatus`, `PushProgress` to `sync_state.rs`. Steps update
-  via `progress.update(cx, |progress, cx| { ...; cx.notify(); })` from the
-  foreground orchestration task; per-server failures carry their reason inline
-  (`Failed("relay did not confirm the event")`).
-- `Backend::push_repository` / `push_checkout` / `create_repository` /
-  `publish_local_repo` accept `Entity<PushProgress>` from the caller.
-- Keep the `pushing_repos` guard and `PushOutcome` returns as-is;
-  `PushProgress` is additive.
-- Validate: unit test in `sync_state.rs` driving `push_to_grasps` with a mock
-  signer against a fake server list, asserting the emitted step sequence.
+  ```
+  Announce repository                          ✓
+  Publish state event → wss://relay.damus.io   ✓
+  Push objects → git.damus.io                  ⟳
+  Push objects → git.ngit.org                  ·
+  ```
 
-### 4. Surface it in the UI
+- Labels come from `PushStep::label()` so user-facing copy lives in one place.
+- The publish dialog (`init_dialog.rs`) can opt in the same way by constructing the `RepoAddr` up front; optional follow-up.
 
-- `RepoStore` holds `push_progress: Option<Entity<PushProgress>>` for the
-  running push. `RepoDetailView` renders a checklist while `pushing`: spinner
-  on `Running`, check on `Done`, cross plus reason on `Failed`. The existing
-  loading state on header/push buttons stays.
-- Dialogs: extend `DialogProgress` with `steps: Option<Entity<PushProgress>>`
-  so the create/publish dialogs show the same checklist.
-- Validate: manual run of the push and create-repository flows; the
-  partial-failure path must still show `render_push_warning_banner` after the
-  checklist.
+Each step compiles green independently: Step 2 adds the methods while the old function still exists, Step 3 switches call sites and deletes it.
 
-Each step lands as a compiling, testable commit: 1 and 2 are mechanical
-refactors, 3 is additive API, 4 is the visible payoff.
+## Validation
 
-## Open decisions
-
-- `PushKind` enum vs. keeping the push closure: the enum matches current call
-  sites; keep the closure if other push modes (e.g. PR namespace pushes) are
-  planned.
-- `PushProgress` as an `Entity` passed in (recommended) vs. a
-  `BackendEvent::PushProgress` broadcast that `RepoStore` accumulates.
+- `cargo check -p signed_state` and `cargo test -p signed_state` after Steps 1 and 2 (the state round-trip tests move with the file).
+- Manual: push a repo with two grasp servers where one is unreachable, and confirm the checklist shows running/✓/✗ per server and the existing `partial_warning` still appears.