From 74e1226b93a79bd9a6b588584085e5486710603d Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Tue, 13 Oct 2026 17:40:53 +0700 Subject: [PATCH] update plan --- docs/push-refactor.md | 251 +++++++++++++++++++++--------------------- 1 file changed, 126 insertions(+), 125 deletions(-) diff --git a/docs/push-refactor.md b/docs/push-refactor.md index 5c6c31a..f1073fb 100644 --- a/docs/push-refactor.md +++ b/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>`. 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, + /// Local repository to push from; `None` for states parsed from events. + pub path: Option, + 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; - 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 { + 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

( &self, - plan: PushPlan, - progress: Entity, 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 + 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(...) -> Result { ... } } ``` -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 -> 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` 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`; 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` 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>` 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>` - 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.