From 90f47159eee49a286ca7a8c70eff79763213d270 Mon Sep 17 00:00:00 2001 From: Mihai Date: Sun, 26 Jul 2026 16:37:52 +0200 Subject: [PATCH] appview: add fork sync button and status Forks had no way to see whether they trailed their upstream, and the sync endpoint was reachable only by hand. Show the relationship next to the "forked from" line and offer a button when a fast-forward is safe. Status is derived in the appview from the knot mirror's existing getBranch and listCommits endpoints, so no new lexicon is needed: compare both heads, then walk the other side's recent history to tell "behind" from "ahead" from "diverged". A diverged fork gets a compare link rather than a button, so the UI never offers an action that would discard commits. Beyond the 100 commit window listCommits allows, the two cases are indistinguishable and both fall through to diverged. The fragment loads over htmx rather than during the repo render, since it costs several mirror calls. On success the handler reports up to date directly instead of re-reading the mirror, which trails the knot by a second or two after a sync. Signed-off-by: Mihai --- appview/pages/forkstatus_test.go | 138 +++++++++++ appview/pages/pages.go | 9 + appview/pages/templates/layouts/repobase.html | 8 + .../templates/repo/fragments/forkStatus.html | 39 ++++ appview/repo/forkstatus.go | 154 ++++++++++++ appview/repo/forkstatus_test.go | 221 ++++++++++++++++++ appview/repo/repo.go | 17 +- appview/repo/router.go | 1 + types/repo.go | 14 ++ 9 files changed, 599 insertions(+), 2 deletions(-) create mode 100644 appview/pages/forkstatus_test.go create mode 100644 appview/pages/templates/repo/fragments/forkStatus.html create mode 100644 appview/repo/forkstatus.go create mode 100644 appview/repo/forkstatus_test.go diff --git a/appview/pages/forkstatus_test.go b/appview/pages/forkstatus_test.go new file mode 100644 index 00000000..fe81a8f2 --- /dev/null +++ b/appview/pages/forkstatus_test.go @@ -0,0 +1,138 @@ +package pages + +import ( + "bytes" + "log/slog" + "strings" + "testing" + + "tangled.org/core/appview/config" + "tangled.org/core/appview/pages/repoinfo" + "tangled.org/core/idresolver" + "tangled.org/core/types" +) + +// The fork status fragment is rendered from a lazily-loaded htmx request, so a +// template error surfaces at runtime rather than at build time. These tests +// parse and execute it for each status the handler can produce. +func renderForkStatus(t *testing.T, status *types.ForkStatusResult) string { + t.Helper() + + p := NewPages(&config.Config{}, idresolver.DefaultResolver(""), nil, nil, slog.Default()) + + var buf bytes.Buffer + err := p.ForkStatusFragment(&buf, ForkStatusParams{ + RepoInfo: repoinfo.RepoInfo{ + OwnerHandle: "alice", + Name: "repo", + }, + Status: status, + }) + if err != nil { + t.Fatalf("rendering fork status fragment: %v", err) + } + + return buf.String() +} + +func TestForkStatusFragment_FastForwardable(t *testing.T) { + out := renderForkStatus(t, &types.ForkStatusResult{ + Status: types.FastForwardable, + Behind: 3, + Branch: "main", + }) + + if !strings.Contains(out, "behind by 3 commits") { + t.Errorf("expected commit count in output, got:\n%s", out) + } + if !strings.Contains(out, "Sync fork") { + t.Errorf("expected sync button in output, got:\n%s", out) + } + if !strings.Contains(out, `value="main"`) { + t.Errorf("expected branch to be posted back, got:\n%s", out) + } +} + +// htmx puts the htmx-request class on whichever element carries hx-post, and +// the compiled selector is `.group\/form.htmx-request .group-[...]\/form:*` -- +// both classes have to land on that same element. Keying the spinner off a +// bare `group` on the button silently never fires. +func TestForkStatusFragment_SpinnerIsWiredToTheForm(t *testing.T) { + out := renderForkStatus(t, &types.ForkStatusResult{ + Status: types.FastForwardable, + Behind: 3, + Branch: "main", + }) + + if !strings.Contains(out, "group/form") { + t.Errorf("form must carry the named group the spinner keys off, got:\n%s", out) + } + if !strings.Contains(out, "group-[.htmx-request]/form:inline") { + t.Errorf("expected a spinner shown during the request, got:\n%s", out) + } + if !strings.Contains(out, "group-[.htmx-request]/form:hidden") { + t.Errorf("expected the idle icon hidden during the request, got:\n%s", out) + } + if !strings.Contains(out, "hx-disabled-elt") { + t.Errorf("expected the button disabled during the request, got:\n%s", out) + } + + // a bare `group` on the button would not match the compiled selector + if strings.Contains(out, `class="btn group text-sm`) { + t.Errorf("spinner keyed off a bare group on the button will never fire, got:\n%s", out) + } +} + +func TestForkStatusFragment_SingularCommit(t *testing.T) { + out := renderForkStatus(t, &types.ForkStatusResult{ + Status: types.FastForwardable, + Behind: 1, + Branch: "main", + }) + + if !strings.Contains(out, "behind by 1 commit") { + t.Errorf("expected singular wording, got:\n%s", out) + } + if strings.Contains(out, "1 commits") { + t.Errorf("expected singular wording, got plural:\n%s", out) + } +} + +func TestForkStatusFragment_Conflict(t *testing.T) { + out := renderForkStatus(t, &types.ForkStatusResult{ + Status: types.Conflict, + Branch: "main", + }) + + if !strings.Contains(out, "diverged") { + t.Errorf("expected divergence notice, got:\n%s", out) + } + if strings.Contains(out, "Sync fork") { + t.Errorf("a diverged fork must not offer a sync button, got:\n%s", out) + } +} + +func TestForkStatusFragment_UpToDate(t *testing.T) { + out := renderForkStatus(t, &types.ForkStatusResult{ + Status: types.UpToDate, + Branch: "main", + }) + + if !strings.Contains(out, "up to date") { + t.Errorf("expected up to date notice, got:\n%s", out) + } + if strings.Contains(out, "Sync fork") { + t.Errorf("an up to date fork must not offer a sync button, got:\n%s", out) + } +} + +func TestForkStatusFragment_MissingBranch(t *testing.T) { + out := renderForkStatus(t, &types.ForkStatusResult{ + Status: types.MissingBranch, + Branch: "main", + }) + + if strings.Contains(out, "Sync fork") { + t.Errorf("a missing branch must not offer a sync button, got:\n%s", out) + } +} diff --git a/appview/pages/pages.go b/appview/pages/pages.go index e20d77af..dddbd6f0 100644 --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -1761,6 +1761,15 @@ func (p *Pages) PipelineStatusesFragment(w io.Writer, params PipelineStatusesPar return p.executePlain("repo/fragments/pipelineStatuses", w, params) } +type ForkStatusParams struct { + RepoInfo repoinfo.RepoInfo + Status *types.ForkStatusResult +} + +func (p *Pages) ForkStatusFragment(w io.Writer, params ForkStatusParams) error { + return p.executePlain("repo/fragments/forkStatus", w, params) +} + type WorkflowParams struct { BaseParams RepoInfo repoinfo.RepoInfo diff --git a/appview/pages/templates/layouts/repobase.html b/appview/pages/templates/layouts/repobase.html index 82f0bd03..dbfd16a8 100644 --- a/appview/pages/templates/layouts/repobase.html +++ b/appview/pages/templates/layouts/repobase.html @@ -81,6 +81,14 @@ {{ $sourceOwner }}/{{ .RepoInfo.Source.Name }} + {{ if .RepoInfo.Roles.IsOwner }} + + {{ end }} {{ end }} {{ end }} diff --git a/appview/pages/templates/repo/fragments/forkStatus.html b/appview/pages/templates/repo/fragments/forkStatus.html new file mode 100644 index 00000000..c6535c89 --- /dev/null +++ b/appview/pages/templates/repo/fragments/forkStatus.html @@ -0,0 +1,39 @@ +{{ define "repo/fragments/forkStatus" }} + {{ $status := .Status }} + {{ $repo := .RepoInfo }} + + {{ if $status.IsFastForwardable }} + + · behind by {{ $status.Behind }} commit{{ if ne $status.Behind 1 }}s{{ end }} + + {{/* the request class lands on the element carrying hx-post, so the + spinner has to key off a named group on the form itself */}} +
+ + +
+ {{ else if $status.IsConflict }} + · has diverged from upstream + + compare + + {{ else if $status.IsUpToDate }} + · up to date + {{ end }} + +{{ end }} diff --git a/appview/repo/forkstatus.go b/appview/repo/forkstatus.go new file mode 100644 index 00000000..eece21b8 --- /dev/null +++ b/appview/repo/forkstatus.go @@ -0,0 +1,154 @@ +package repo + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + "tangled.org/core/api/tangled" + "tangled.org/core/appview/pages" + "tangled.org/core/types" +) + +// forkStatusWindow caps how far back we walk a branch's history looking for the +// other side's head. Past this we can't tell "very far behind" from "diverged", +// so we report neither and point the user at the compare view instead. +// +// The mirror's listCommits lexicon caps limit at 100. +const forkStatusWindow = 100 + +// ForkStatus compares a fork's branch against the same branch on its upstream +// source and renders a fragment describing the difference. +// +// This is loaded lazily over htmx rather than computed during the repo page +// render: it costs two to four calls to the knot mirror, and the mirror trails +// the knot by a second or two after a push. +func (rp *Repo) ForkStatus(w http.ResponseWriter, r *http.Request) { + l := rp.logger.With("handler", "ForkStatus") + + f, err := rp.repoResolver.Resolve(r) + if err != nil { + l.Error("failed to resolve repo", "err", err) + return + } + + user := rp.oauth.GetMultiAccountUser(r) + repoInfo := rp.repoResolver.GetRepoInfo(r, user) + + // nothing to compare against + if repoInfo.Source == nil { + return + } + + branch := r.URL.Query().Get("branch") + if branch == "" { + branch, err = rp.defaultBranch(r.Context(), f.RepoDid) + if err != nil { + l.Warn("failed to resolve default branch", "err", err) + return + } + } + + status, err := rp.compareFork(r.Context(), f.RepoDid, repoInfo.Source.RepoDid, branch) + if err != nil { + // A fork the mirror hasn't cloned yet is the common case here, and a + // fresh fork is up to date with its source by construction. Render + // nothing rather than an error. + l.Warn("failed to compare fork against source", "err", err) + return + } + + if err := rp.pages.ForkStatusFragment(w, pages.ForkStatusParams{ + RepoInfo: repoInfo, + Status: status, + }); err != nil { + l.Error("failed to render fork status", "err", err) + } +} + +// compareFork determines how a fork's branch relates to the same branch on its +// source, using only the mirror's existing read endpoints. +func (rp *Repo) compareFork(ctx context.Context, forkDid, sourceDid, branch string) (*types.ForkStatusResult, error) { + result := &types.ForkStatusResult{Branch: branch} + + forkHash, err := rp.branchHash(ctx, forkDid, branch) + if err != nil { + return nil, fmt.Errorf("resolving fork branch: %w", err) + } + + sourceHash, err := rp.branchHash(ctx, sourceDid, branch) + if err != nil { + // the source may simply not carry this branch + result.Status = types.MissingBranch + return result, nil + } + + if forkHash == sourceHash { + result.Status = types.UpToDate + return result, nil + } + + // Is the fork's head in the source's recent history? If so the fork is + // strictly behind and a fast-forward is safe. + if behind, found, err := rp.distanceTo(ctx, sourceDid, branch, forkHash); err != nil { + return nil, err + } else if found { + result.Status = types.FastForwardable + result.Behind = behind + return result, nil + } + + // Otherwise, is the source's head in the fork's recent history? Then the + // fork is ahead and there is nothing to pull in. + if _, found, err := rp.distanceTo(ctx, forkDid, branch, sourceHash); err != nil { + return nil, err + } else if found { + result.Status = types.UpToDate + return result, nil + } + + // Neither head appears in the other's window: the branches have diverged, + // or drifted further apart than we looked. Either way a plain sync is not + // safe, so offer a comparison rather than a button. + result.Status = types.Conflict + return result, nil +} + +// branchHash returns the commit hash at the tip of a branch. +func (rp *Repo) branchHash(ctx context.Context, repoDid, branch string) (string, error) { + out, err := tangled.GitTempGetBranch(ctx, rp.knotMirrorXRPCClient(), branch, repoDid) + if err != nil { + return "", err + } + return out.Hash, nil +} + +// distanceTo walks a branch's recent history looking for target, returning how +// many commits precede it. found is false if target isn't within the window. +func (rp *Repo) distanceTo(ctx context.Context, repoDid, branch, target string) (int, bool, error) { + xrpcBytes, err := tangled.GitTempListCommits( + ctx, + rp.knotMirrorXRPCClient(), + "", + forkStatusWindow, + branch, + repoDid, + ) + if err != nil { + return 0, false, fmt.Errorf("listing commits: %w", err) + } + + var resp types.RepoLogResponse + if err := json.Unmarshal(xrpcBytes, &resp); err != nil { + return 0, false, fmt.Errorf("decoding commits: %w", err) + } + + for i, c := range resp.Commits { + if c.Hash.String() == target { + return i, true, nil + } + } + + return 0, false, nil +} diff --git a/appview/repo/forkstatus_test.go b/appview/repo/forkstatus_test.go new file mode 100644 index 00000000..3e18921b --- /dev/null +++ b/appview/repo/forkstatus_test.go @@ -0,0 +1,221 @@ +package repo + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/bluesky-social/indigo/util" + indigoxrpc "github.com/bluesky-social/indigo/xrpc" + "github.com/go-git/go-git/v5/plumbing" + "tangled.org/core/types" +) + +const ( + forkDid = "did:plc:fork" + sourceDid = "did:plc:source" + branch = "main" +) + +// fakeMirror stands in for knotmirror, serving the two read endpoints that fork +// status is derived from. Responses are built from types.RepoLogResponse and +// marshalled here so the wire encoding matches what the real handler produces +// (notably plumbing.Hash, which is a byte array rather than a hex string). +type fakeMirror struct { + // heads maps a repo DID to the commit at the tip of branch. A missing entry + // makes getBranch fail, as it does when the branch isn't there. + heads map[string]string + // history maps a repo DID to that branch's commits, newest first. + history map[string][]string +} + +func (f *fakeMirror) server(t *testing.T) *httptest.Server { + t.Helper() + + mux := http.NewServeMux() + + mux.HandleFunc("/xrpc/sh.tangled.git.temp.getBranch", func(w http.ResponseWriter, r *http.Request) { + repo := r.URL.Query().Get("repo") + head, ok := f.heads[repo] + if !ok { + http.Error(w, `{"error":"BranchNotFound"}`, http.StatusNotFound) + return + } + json.NewEncoder(w).Encode(map[string]any{ + "name": r.URL.Query().Get("name"), + "hash": head, + "when": "2026-01-01T00:00:00Z", + }) + }) + + mux.HandleFunc("/xrpc/sh.tangled.git.temp.listCommits", func(w http.ResponseWriter, r *http.Request) { + repo := r.URL.Query().Get("repo") + var commits []types.Commit + for _, h := range f.history[repo] { + commits = append(commits, types.Commit{Hash: plumbing.NewHash(h)}) + } + json.NewEncoder(w).Encode(types.RepoLogResponse{Commits: commits}) + }) + + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +func (f *fakeMirror) repo(t *testing.T) *Repo { + t.Helper() + srv := f.server(t) + return &Repo{ + knotMirrorXRPC: &indigoxrpc.Client{ + Host: srv.URL, + Client: util.RobustHTTPClient(), + }, + } +} + +// sha builds a distinguishable 40-char hex hash from a short label. +func sha(label string) string { + return strings.Repeat(label, 40)[:40] +} + +func TestCompareFork(t *testing.T) { + var ( + base = sha("a") + mid = sha("b") + tip = sha("c") + alt = sha("d") + ) + + tests := []struct { + name string + mirror fakeMirror + wantStatus types.ForkStatus + wantBehind int + }{ + { + name: "identical heads are up to date", + mirror: fakeMirror{ + heads: map[string]string{forkDid: tip, sourceDid: tip}, + }, + wantStatus: types.UpToDate, + }, + { + name: "fork strictly behind is fast forwardable", + mirror: fakeMirror{ + heads: map[string]string{forkDid: base, sourceDid: tip}, + // source history newest first: tip, mid, base + history: map[string][]string{sourceDid: {tip, mid, base}}, + }, + wantStatus: types.FastForwardable, + wantBehind: 2, + }, + { + name: "fork behind by one", + mirror: fakeMirror{ + heads: map[string]string{forkDid: mid, sourceDid: tip}, + history: map[string][]string{sourceDid: {tip, mid}}, + }, + wantStatus: types.FastForwardable, + wantBehind: 1, + }, + { + name: "fork ahead of source has nothing to pull", + mirror: fakeMirror{ + heads: map[string]string{forkDid: tip, sourceDid: base}, + // source doesn't contain the fork's head... + history: map[string][]string{ + sourceDid: {base}, + // ...but the fork contains the source's head + forkDid: {tip, mid, base}, + }, + }, + wantStatus: types.UpToDate, + }, + { + name: "diverged branches conflict", + mirror: fakeMirror{ + heads: map[string]string{forkDid: alt, sourceDid: tip}, + history: map[string][]string{ + sourceDid: {tip, mid, base}, + forkDid: {alt, base}, + }, + }, + wantStatus: types.Conflict, + }, + { + name: "source missing the branch", + mirror: fakeMirror{ + heads: map[string]string{forkDid: tip}, + }, + wantStatus: types.MissingBranch, + }, + { + name: "further apart than the window conflicts", + mirror: fakeMirror{ + heads: map[string]string{forkDid: base, sourceDid: tip}, + // neither history mentions the other's head + history: map[string][]string{ + sourceDid: {tip, mid}, + forkDid: {base}, + }, + }, + wantStatus: types.Conflict, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rp := tt.mirror.repo(t) + + got, err := rp.compareFork(context.Background(), forkDid, sourceDid, branch) + if err != nil { + t.Fatalf("compareFork: %v", err) + } + + if got.Status != tt.wantStatus { + t.Errorf("status = %v, want %v", got.Status, tt.wantStatus) + } + if got.Status == types.FastForwardable && got.Behind != tt.wantBehind { + t.Errorf("behind = %d, want %d", got.Behind, tt.wantBehind) + } + if got.Branch != branch { + t.Errorf("branch = %q, want %q", got.Branch, branch) + } + }) + } +} + +func TestCompareFork_ForkBranchMissingIsAnError(t *testing.T) { + // the fork not having the branch is different from the source not having + // it: we can't say anything useful, so the caller renders nothing + m := fakeMirror{heads: map[string]string{sourceDid: sha("c")}} + rp := m.repo(t) + + if _, err := rp.compareFork(context.Background(), forkDid, sourceDid, branch); err == nil { + t.Error("expected an error when the fork lacks the branch") + } +} + +// A diverged fork must never be reported as fast-forwardable: that is the one +// mistake that would let the UI offer a button which discards commits. +func TestCompareFork_DivergedIsNeverFastForwardable(t *testing.T) { + m := fakeMirror{ + heads: map[string]string{forkDid: sha("d"), sourceDid: sha("c")}, + history: map[string][]string{ + sourceDid: {sha("c"), sha("b")}, + forkDid: {sha("d"), sha("b")}, + }, + } + rp := m.repo(t) + + got, err := rp.compareFork(context.Background(), forkDid, sourceDid, branch) + if err != nil { + t.Fatalf("compareFork: %v", err) + } + if got.Status == types.FastForwardable { + t.Fatal("diverged fork reported as fast forwardable; sync button would discard commits") + } +} diff --git a/appview/repo/repo.go b/appview/repo/repo.go index f899b9d3..2ea5f7a9 100644 --- a/appview/repo/repo.go +++ b/appview/repo/repo.go @@ -1419,7 +1419,7 @@ func (rp *Repo) SyncRepoFork(w http.ResponseWriter, r *http.Request) { client, &tangled.RepoForkSync_Input{ Repo: f.RepoDid, - Did: &user.Did, + Did: &f.Did, Name: &f.Name, Source: &f.Source, Branch: ref, @@ -1431,7 +1431,20 @@ func (rp *Repo) SyncRepoFork(w http.ResponseWriter, r *http.Request) { return } - rp.pages.HxRefresh(w) + // Report success straight away rather than re-reading the status. The + // knot is authoritative and has accepted the fast-forward, but the knot + // mirror we'd read back from trails it by a second or two, so asking + // again here would usually still say "behind". + repoInfo := rp.repoResolver.GetRepoInfo(r, user) + if err := rp.pages.ForkStatusFragment(w, pages.ForkStatusParams{ + RepoInfo: repoInfo, + Status: &types.ForkStatusResult{ + Status: types.UpToDate, + Branch: ref, + }, + }); err != nil { + l.Error("failed to render fork status", "err", err) + } return } } diff --git a/appview/repo/router.go b/appview/repo/router.go index c3b053ba..fe796d8f 100644 --- a/appview/repo/router.go +++ b/appview/repo/router.go @@ -54,6 +54,7 @@ func (rp *Repo) Router(mw *middleware.Middleware) http.Handler { r.Use(middleware.AuthMiddleware(rp.oauth)) r.Get("/", rp.ForkRepo) r.Post("/", rp.ForkRepo) + r.With(mw.RepoPermissionMiddleware("repo:owner")).Get("/status", rp.ForkStatus) r.With(mw.RepoPermissionMiddleware("repo:owner")).Route("/sync", func(r chi.Router) { r.Post("/", rp.SyncRepoFork) }) diff --git a/types/repo.go b/types/repo.go index 58bd999e..a1e5513e 100644 --- a/types/repo.go +++ b/types/repo.go @@ -95,6 +95,20 @@ type ForkInfo struct { Status ForkStatus } +// ForkStatusResult describes how a fork's branch relates to the same branch on +// its upstream source. +type ForkStatusResult struct { + Status ForkStatus + // Behind is how many commits the fork trails the source by, and is only + // meaningful when Status is FastForwardable. + Behind int + Branch string +} + +func (f ForkStatusResult) IsUpToDate() bool { return f.Status == UpToDate } +func (f ForkStatusResult) IsFastForwardable() bool { return f.Status == FastForwardable } +func (f ForkStatusResult) IsConflict() bool { return f.Status == Conflict } + type RepoLanguageDetails struct { Name string Percentage float32 -- 2.54.0