From 5d4a421d1c47567e91870b23b9d98c6896a68c21 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Tue, 13 Oct 2026 16:09:25 +0700 Subject: [PATCH] add plan --- docs/TODO.md | 1 + docs/push-refactor.md | 164 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 docs/push-refactor.md diff --git a/docs/TODO.md b/docs/TODO.md index fd9183b..57f8b40 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -28,6 +28,7 @@ ## Push +- [ ] Refactor the push pipeline onto `SyncState` with step-by-step progress, see [push-refactor.md](./push-refactor.md). - [ ] Add a fast-forward check and a force option (nak refuses non-FF without `--force`), today it relies entirely on grasp CAS. - [ ] Allow NIP-34 maintainers to push, not just the owner. diff --git a/docs/push-refactor.md b/docs/push-refactor.md new file mode 100644 index 0000000..5c6c31a --- /dev/null +++ b/docs/push-refactor.md @@ -0,0 +1,164 @@ +# Push refactor plan + +Refactor the grasp push pipeline into `SyncState` methods with step-by-step +progress the UI can render while a push is in flight. + +## Motivation + +- `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. + +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. + +## Target shape + +```rust +// signed_state/src/sync_state.rs +pub struct SyncState { + pub refs: Vec<(String, String)>, + pub head: Option, +} + +pub enum PushStep { + ReadLocalState, + Announce, + StageStateEvent(RelayUrl), + PushGitObjects(RelayUrl), + PublishStateEvent, +} + +pub enum StepStatus { + Running, + Done, + Failed(String), +} + +pub struct PushProgress { + pub steps: Vec<(PushStep, StepStatus)>, +} +``` + +`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( + &self, + plan: PushPlan, + progress: Entity, + cx: &mut AsyncApp, + ) -> PushOutcome; + async fn stage_on_grasp(...) -> Result<(), String>; // connect + purgatory staging + async fn publish(...); // final broadcast +} +``` + +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. + +Flow: + +``` +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 +``` + +## Steps + +### 1. Move and rename (mechanical) + +- `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`. + +### 2. Consolidate the push core onto `SyncState` (behavior-preserving) + +- 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`). + +### 3. Add step-by-step progress + +- 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. + +### 4. Surface it in the UI + +- `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 lands as a compiling, testable commit: 1 and 2 are mechanical +refactors, 3 is additive API, 4 is the visible payoff. + +## 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.