Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

Let an author move their change request's head from the browser

53ff967be619894b62ca0ca69cd9a9f5f52bce01 · Keenan Nemetz · 13d ago · parent 8e1005eb

Let an author move their change request's head from the browser

A request names the exact commit under review, so a fixup pushed after it
was opened is invisible until the head moves. The only mover was
`vole change update --head`, which writes the metadata branch — a write a
contributor cannot make. The person who owns the branch was the one person
who could not keep the request current.

The request page now offers the author (or a maintainer) a branch picker
that moves an open request's head to that branch's tip. It applies the
same rule the new-request form does: the head must be a branch tip, and a
contributor may only name a branch under their own namespace, which is
what the pre-receive hook would have let them push. Approvals stay in the
tree as history and stop matching, which is what a new head already means.

SetRequestHead now refuses a closed or merged request, so the CLI and the
browser share that rule instead of the web handler owning it alone.

server/auth_test.go +130

@@ -6,6 +6,7 @@ import (
6 6 "io"
7 7 "net/http"
8 8 "net/url"
9 "os"
9 10 "path/filepath"
10 11 "strings"
11 12 "testing"
@@ -373,6 +374,135 @@ func TestContributorNamespaceAndRequestUseTheAccountName(t *testing.T) {
373 374 }
374 375 }
375 376
377 // TestContributorMovesTheirOwnRequestHead: a request names a commit, not a branch,
378 // so a fixup pushed after it was opened is invisible until the head moves. The CLI
379 // can move it, but only by writing the metadata branch a contributor cannot push,
380 // so the browser has to offer the same move — under the same namespace rule the
381 // create form uses, and only while the request is open.
382 func TestContributorMovesTheirOwnRequestHead(t *testing.T) {
383 t.Parallel()
384 f := browserFixture(t)
385
386 pub, priv := keypair(t, filepath.Join(f.cfg.DataDir, "id_contrib_head"))
387 contrib, err := f.srv.Identity.Create("contrib", pub, "test")
388 if err != nil {
389 t.Fatal(err)
390 }
391 m := mustRepo(t, f)
392 a, err := m.ACL()
393 if err != nil {
394 t.Fatal(err)
395 }
396 if err := a.SetEntry(contrib.Name, "contribute"); err != nil {
397 t.Fatal(err)
398 }
399 if err := m.SetACL(a, "test"); err != nil {
400 t.Fatal(err)
401 }
402
403 base := workRepo(t, map[string]string{"readme.md": "base\n"})
404 if _, errs, err := f.git(base, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil {
405 t.Fatalf("owner push: %v\n%s", err, errs)
406 }
407 work := workRepo(t, map[string]string{"readme.md": "contribution\n"})
408 headRef := "contrib/" + contrib.Name + "/fix"
409 if _, errs, err := f.gitWithKey(priv, work, "push", f.sshURL(f.repo), "main:refs/heads/"+headRef); err != nil {
410 t.Fatalf("contributor push: %v\n%s", err, errs)
411 }
412
413 sess, cookie := f.browserSession(t, contrib.Name)
414 resp, body := f.browserPost(t, "/r/"+f.repo+"/requests", cookie, url.Values{
415 "csrf": {sess.CSRF}, "title": {"Contributor patch"}, "base": {"main"}, "head": {headRef},
416 })
417 if resp.StatusCode != http.StatusSeeOther {
418 t.Fatalf("open: status=%d body=%s", resp.StatusCode, body)
419 }
420 list, err := metadata.LoadRequests(m.Git(), metadata.Ref)
421 if err != nil || len(list) != 1 {
422 t.Fatalf("requests = %v, %v", list, err)
423 }
424 id := list[0].ID
425 first := list[0].Head
426
427 // A fixup lands on the contributor's branch; the request still names the old
428 // commit until somebody moves it.
429 if err := os.WriteFile(filepath.Join(work, "readme.md"), []byte("contribution v2\n"), 0o644); err != nil {
430 t.Fatal(err)
431 }
432 if _, errs, err := runGit(t, work, "commit", "-qam", "fixup"); err != nil {
433 t.Fatalf("commit: %s", errs)
434 }
435 if _, errs, err := f.gitWithKey(priv, work, "push", f.sshURL(f.repo), "HEAD:refs/heads/"+headRef); err != nil {
436 t.Fatalf("contributor push 2: %v\n%s", err, errs)
437 }
438 second, err := m.Git().RevParse("refs/heads/" + headRef)
439 if err != nil || second == first {
440 t.Fatalf("second tip = %q, %v (first %s)", second, err, first)
441 }
442
443 // The control is offered, with the contributor's own branch as a choice.
444 resp, body = f.browserGet(t, "/r/"+f.repo+"/requests/"+id, cookie)
445 if resp.StatusCode != http.StatusOK {
446 t.Fatalf("request page: status=%d body=%s", resp.StatusCode, body)
447 }
448 if !strings.Contains(body, `action="/r/`+f.repo+`/requests/`+id+`/head"`) || !strings.Contains(body, headRef) {
449 t.Fatalf("the request page does not offer to move the head:\n%s", body)
450 }
451
452 resp, body = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/head", cookie, url.Values{
453 "csrf": {sess.CSRF}, "head": {headRef},
454 })
455 if resp.StatusCode != http.StatusSeeOther {
456 t.Fatalf("move head: status=%d body=%s", resp.StatusCode, body)
457 }
458 got, err := metadata.GetRequest(m.Git(), id)
459 if err != nil {
460 t.Fatal(err)
461 }
462 if got.Head != second {
463 t.Fatalf("head = %s, want the branch tip %s", got.Head, second)
464 }
465
466 // The namespace rule is the one the create form applies: a contributor cannot
467 // point their request at a branch they could not have pushed.
468 resp, _ = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/head", cookie, url.Values{
469 "csrf": {sess.CSRF}, "head": {"main"},
470 })
471 if resp.StatusCode != http.StatusForbidden {
472 t.Fatalf("contributor moved the head to main: status=%d", resp.StatusCode)
473 }
474 if after, _ := metadata.GetRequest(m.Git(), id); after.Head != second {
475 t.Fatalf("head moved to %s despite the refusal", after.Head)
476 }
477
478 // A maintainer may move any open request, including a contributor's.
479 ownerSess, ownerCookie := f.browserSession(t, f.ownerID)
480 mainTip, _ := m.Git().RevParse("refs/heads/main")
481 resp, body = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/head", ownerCookie, url.Values{
482 "csrf": {ownerSess.CSRF}, "head": {"main"},
483 })
484 if resp.StatusCode != http.StatusSeeOther {
485 t.Fatalf("maintainer move head: status=%d body=%s", resp.StatusCode, body)
486 }
487 if got, _ := metadata.GetRequest(m.Git(), id); got.Head != mainTip {
488 t.Fatalf("head = %s, want the maintainer's choice %s", got.Head, mainTip)
489 }
490
491 // Closed requests are settled: the head is history, not a setting.
492 resp, body = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/state", cookie, url.Values{
493 "csrf": {sess.CSRF}, "state": {"closed"},
494 })
495 if resp.StatusCode != http.StatusSeeOther {
496 t.Fatalf("close: status=%d body=%s", resp.StatusCode, body)
497 }
498 resp, _ = f.browserPost(t, "/r/"+f.repo+"/requests/"+id+"/head", cookie, url.Values{
499 "csrf": {sess.CSRF}, "head": {headRef},
500 })
501 if resp.StatusCode != http.StatusForbidden {
502 t.Fatalf("a closed request moved its head: status=%d", resp.StatusCode)
503 }
504 }
505
376 506 func TestContributorCIIsHeldUntilWriterAllows(t *testing.T) {
377 507 t.Parallel()
378 508 f := newFixtureWith(t, func(c *config.Config) { c.LeaseSeconds = 2 })