add plan

2026-10-13 16:09:25 +07:00
parent b78418d815
commit 5d4a421d1c
2 changed files with 165 additions and 0 deletions
@@ -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<String>,
+}
+
+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<Self>;
+    pub async fn push_to_grasps(
+        &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
+}
+```
+
+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<PushProgress> -> 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<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.
+
+### 4. Surface it in the UI
+
+- `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 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.