vole
Clone
Read only, no credentials:
https://rodent.software/vole.git
Write over SSH with a registered key:
git clone ssh://git@rodent.software:2222/vole.git
A free and open-source forge that lives in your Git repo.
Phase 7: the CLI knows who you are and where you wrote it
a63475ce8a689107f6572150d7c58be1d623f6f0 · Keenan Nemetz · 17d ago · parent 79c88d72
Phase 7: the CLI knows who you are and where you wrote it Three things a first-time user hit in a row, all measured against a live instance before they were fixed. 1. Identity was guessed. Every issue and request was stamped with a name from `git config user.name`, sanitised: "Keenan Nemetz" became `keenan-nemetz`, an account that does not exist, and nothing complained — the issue page showed it, and the assignee rule accepted it because that rule only checks the shape. `vole-whoami` is a new SSH verb, and `vole whoami` is the CLI command that reads it: the instance is the only place a key maps to an account, and it is one call on the connection already open. `vole issue` and `vole request` stamp the account's current name and id, and `--author` is gone from both, because a name you can type is a name you can get wrong. Read commands still need no identity — `issue list` and `issue show` keep working with the instance unreachable — and the git-config guess survives only as a warned fallback for `--no-push`. 2. `--remote ssh://…` did not fetch. `fetch` checked for a configured remote name, so a URL was skipped: the first write in a fresh repository succeeded (nothing to fast-forward from) and every later one pushed from a tip that had never been updated, was rejected, and then retried as a "lost race" that never happened, reporting `already exists`. A URL is now fetched like a named remote, so `git init` plus a URL is the real no-clone filing path the roadmap described. 3. A command answered with an id and stopped. `new` and `comment` now print the page they wrote, and a comment link lands on that comment: the instance reports its own `public_url` in the same whoami reply (new optional config key), and the web puts an `id="c-<ulid>"` anchor on every comment. Empty states on the issues and requests pages print the command that fills them, because four of the five things a reader wants to do are writes the web cannot take, and "no comments yet" is a dead end. Verified live, not only in tests: `vole whoami` in a clone whose `user.name` is "Keenan Nemetz" printed `keenan` while the issue it then filed carried `author: keenan` on the branch; two repositories that never clone both wrote through the URL (the exact case that failed before); and a comment printed a `#c-01M5N3M0…` permalink whose anchor is on the served page. Tests: TestWhoAmIAnswersFromTheInstance (including `--author` now being an unknown option), TestReadCommandsNeedNoIdentity (closes the SSH listener, then lists), TestRequestAuthorComesFromTheKey, TestAURLRemoteIsFetchedBeforeWriting, TestEmptyStatesPrintAWorkingCommand, and the comment-empty-state assertion added to TestIssuePushedThenRendered. scripts/ci.sh green (gofmt, vet, build, markdown links, go test -race ./...).
internal/server/integration_test.go +194−29
| @@ -303,6 +303,22 @@ func (f *fixture) get(path string) (int, string) { | |||
| 303 | 303 | return resp.StatusCode, string(body) | |
| 304 | 304 | } | |
| 305 | 305 | ||
| 306 | // cli runs the built binary in dir as the owner. A null global git config means | ||
| 307 | // a developer's own identity cannot change what a test sees. | ||
| 308 | func (f *fixture) cli(dir string, args ...string) (string, string, error) { | ||
| 309 | f.t.Helper() | ||
| 310 | cmd := exec.Command(binPath, args...) | ||
| 311 | cmd.Dir = dir | ||
| 312 | cmd.Env = append(os.Environ(), | ||
| 313 | "GIT_SSH_COMMAND=ssh -i "+f.keyPath+" -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR", | ||
| 314 | "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null", | ||
| 315 | "USER=keenan") | ||
| 316 | var out, errb bytes.Buffer | ||
| 317 | cmd.Stdout, cmd.Stderr = &out, &errb | ||
| 318 | err := cmd.Run() | ||
| 319 | return out.String(), errb.String(), err | ||
| 320 | } | ||
| 321 | |||
| 306 | 322 | // TestPushOverSSHThenReadOverBothTransports is the core loop: writes need a key, | |
| 307 | 323 | // reads do not. | |
| 308 | 324 | func TestPushOverSSHThenReadOverBothTransports(t *testing.T) { | |
| @@ -554,6 +570,11 @@ func TestIssuePushedThenRendered(t *testing.T) { | |||
| 554 | 570 | if !strings.Contains(body, "<code>code</code>") { | |
| 555 | 571 | t.Fatal("inline code was not rendered") | |
| 556 | 572 | } | |
| 573 | // An empty thread prints the command that fills it: the web is read-only, so | ||
| 574 | // telling somebody "no comments" and stopping is a dead end. | ||
| 575 | if !strings.Contains(body, "vole issue comment "+testIssueID) { | ||
| 576 | t.Fatalf("the empty comment state does not print the command:\n%s", body) | ||
| 577 | } | ||
| 557 | 578 | // The repo page advertises the count. | |
| 558 | 579 | if _, body := f.get("/r/" + f.repo); !strings.Contains(body, "Issues") { | |
| 559 | 580 | t.Fatal("repo page has no issues link") | |
| @@ -827,12 +848,13 @@ func TestIssueCLIEndToEnd(t *testing.T) { | |||
| 827 | 848 | } | |
| 828 | 849 | } | |
| 829 | 850 | ||
| 830 | // TestRequestCLIResolvesNamesOverSSH: the roster is server-side SQLite, so a clone | ||
| 831 | // has no copy of it to read. `vole request new --author <name>` has to ask the | ||
| 832 | // instance over the SSH channel it pushes on, and the request it writes must carry | ||
| 833 | // the account id the server authenticated rather than the name it was given. | ||
| 834 | func TestRequestCLIResolvesNamesOverSSH(t *testing.T) { | ||
| 835 | f := newFixture(t) | ||
| 851 | // TestWhoAmIAnswersFromTheInstance is the fix for a name guessed from git config. | ||
| 852 | // `user.name` on a real developer machine is "Keenan Nemetz", which sanitises to | ||
| 853 | // `keenan-nemetz` — an account that does not exist — and the old CLI stamped that | ||
| 854 | // on every issue it filed, silently. The key is the identity; the instance is the | ||
| 855 | // only thing that knows which account it belongs to. | ||
| 856 | func TestWhoAmIAnswersFromTheInstance(t *testing.T) { | ||
| 857 | f := newFixtureWith(t, func(c *config.Config) { c.PublicURL = "http://forge.test" }) | ||
| 836 | 858 | f.publish(f.repo) | |
| 837 | 859 | work := workRepo(t, map[string]string{"readme.md": "hi\n"}) | |
| 838 | 860 | if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil { | |
| @@ -842,26 +864,116 @@ func TestRequestCLIResolvesNamesOverSSH(t *testing.T) { | |||
| 842 | 864 | if _, errs, err := f.git("", "clone", "-q", f.sshURL(f.repo), clone); err != nil { | |
| 843 | 865 | t.Fatalf("clone: %s", errs) | |
| 844 | 866 | } | |
| 867 | // The trap, exactly as it looks on a normal machine. | ||
| 868 | if _, errs, err := runGit(t, clone, "config", "user.name", "Keenan Nemetz"); err != nil { | ||
| 869 | t.Fatalf("config: %s", errs) | ||
| 870 | } | ||
| 845 | 871 | ||
| 846 | run := func(args ...string) (string, string, error) { | ||
| 847 | cmd := exec.Command(binPath, args...) | ||
| 848 | cmd.Dir = clone | ||
| 849 | cmd.Env = append(os.Environ(), | ||
| 850 | "GIT_SSH_COMMAND=ssh -i "+f.keyPath+" -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR", | ||
| 851 | "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null", | ||
| 852 | "USER=keenan") | ||
| 853 | var out, errb bytes.Buffer | ||
| 854 | cmd.Stdout, cmd.Stderr = &out, &errb | ||
| 855 | err := cmd.Run() | ||
| 856 | return out.String(), errb.String(), err | ||
| 872 | out, errs, err := f.cli(clone, "whoami") | ||
| 873 | if err != nil { | ||
| 874 | t.Fatalf("whoami: %v\n%s", err, errs) | ||
| 875 | } | ||
| 876 | if got := strings.TrimSpace(out); got != "keenan" { | ||
| 877 | t.Fatalf("whoami = %q, want the account name keenan", got) | ||
| 878 | } | ||
| 879 | out, _, err = f.cli(clone, "whoami", "--id") | ||
| 880 | if err != nil { | ||
| 881 | t.Fatalf("whoami --id: %v", err) | ||
| 882 | } | ||
| 883 | if got := strings.TrimSpace(out); got != f.ownerID { | ||
| 884 | t.Fatalf("whoami --id = %q, want %q", got, f.ownerID) | ||
| 885 | } | ||
| 886 | |||
| 887 | // A filed issue is authored by the account, not by the guess, and the command | ||
| 888 | // says where to read it. | ||
| 889 | out, errs, err = f.cli(clone, "issue", "new", "--title", "who am I") | ||
| 890 | if err != nil { | ||
| 891 | t.Fatalf("issue new: %v\n%s", err, errs) | ||
| 892 | } | ||
| 893 | id := strings.Fields(out)[0] | ||
| 894 | raw, _, err := runGit(t, filepath.Join(f.cfg.ReposDir(), f.repo+".git"), | ||
| 895 | "show", "refs/heads/vole:issues/"+id+"/issue.md") | ||
| 896 | if err != nil { | ||
| 897 | t.Fatalf("read the issue from the server: %v\n%s", err, raw) | ||
| 898 | } | ||
| 899 | if !strings.Contains(raw, "author: keenan\n") { | ||
| 900 | t.Fatalf("the issue was not authored by the account:\n%s", raw) | ||
| 901 | } | ||
| 902 | if strings.Contains(raw, "keenan-nemetz") { | ||
| 903 | t.Fatalf("the git-config guess reached the branch:\n%s", raw) | ||
| 904 | } | ||
| 905 | want := "http://forge.test/r/" + f.repo + "/issues/" + id | ||
| 906 | if !strings.Contains(errs, want) { | ||
| 907 | t.Fatalf("the CLI did not print where to read it (%q):\n%s", want, errs) | ||
| 908 | } | ||
| 909 | |||
| 910 | // A comment prints a permalink that resolves: the page carries the anchor. | ||
| 911 | out, errs, err = f.cli(clone, "issue", "comment", id, "-m", "a reply") | ||
| 912 | if err != nil { | ||
| 913 | t.Fatalf("comment: %v\n%s", err, errs) | ||
| 914 | } | ||
| 915 | cid := strings.TrimSpace(out) | ||
| 916 | if !strings.Contains(errs, want+"#c-"+cid) { | ||
| 917 | t.Fatalf("the comment permalink is wrong:\n%s", errs) | ||
| 918 | } | ||
| 919 | _, page := f.get("/r/" + f.repo + "/issues/" + id) | ||
| 920 | if !strings.Contains(page, `id="c-`+cid+`"`) { | ||
| 921 | t.Fatalf("the page has no anchor for %s:\n%s", cid, page) | ||
| 922 | } | ||
| 923 | |||
| 924 | // The typed name is gone: a name you can type is a name you can get wrong. | ||
| 925 | if _, errs, err := f.cli(clone, "issue", "new", "--title", "x", "--author", "wilbur"); err == nil { | ||
| 926 | t.Fatalf("--author is still accepted: %s", errs) | ||
| 927 | } | ||
| 928 | } | ||
| 929 | |||
| 930 | // TestReadCommandsNeedNoIdentity: `issue list` and `issue show` read the branch, | ||
| 931 | // so they must keep working where the instance cannot be reached at all. | ||
| 932 | func TestReadCommandsNeedNoIdentity(t *testing.T) { | ||
| 933 | f := newFixture(t) | ||
| 934 | f.publish(f.repo) | ||
| 935 | work := workRepo(t, map[string]string{"readme.md": "hi\n"}) | ||
| 936 | if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil { | ||
| 937 | t.Fatalf("push main: %s", errs) | ||
| 857 | 938 | } | |
| 939 | clone := filepath.Join(t.TempDir(), "clone") | ||
| 940 | if _, errs, err := f.git("", "clone", "-q", f.sshURL(f.repo), clone); err != nil { | ||
| 941 | t.Fatalf("clone: %s", errs) | ||
| 942 | } | ||
| 943 | if out, errs, err := f.cli(clone, "issue", "new", "--title", "offline read", "-m", "body"); err != nil { | ||
| 944 | t.Fatalf("issue new: %v (%s)", err, errs) | ||
| 945 | } else if fields := strings.Fields(out); len(fields) > 0 { | ||
| 946 | // The instance goes away; the branch is still here. | ||
| 947 | f.srv.SSH.Close() | ||
| 948 | out, errs, err := f.cli(clone, "issue", "list") | ||
| 949 | if err != nil { | ||
| 950 | t.Fatalf("issue list needs the instance but should not: %v\n%s", err, errs) | ||
| 951 | } | ||
| 952 | if !strings.Contains(out, "offline read") { | ||
| 953 | t.Fatalf("issue list lost the issue:\n%s", out) | ||
| 954 | } | ||
| 955 | } | ||
| 956 | } | ||
| 858 | 957 | ||
| 859 | // A name the roster does not know fails before anything is pushed. | ||
| 860 | if _, errs, err := run("request", "new", "--title", "nobody", "-m", "x", "--author", "nobody"); err == nil { | ||
| 861 | t.Fatalf("an unknown author was accepted: %s", errs) | ||
| 958 | // TestRequestAuthorComesFromTheKey: a request records the account that opened it, | ||
| 959 | // and there is no flag for saying otherwise. `--author` was a name you could typo | ||
| 960 | // into a round trip; the key already answers the question. | ||
| 961 | func TestRequestAuthorComesFromTheKey(t *testing.T) { | ||
| 962 | f := newFixture(t) | ||
| 963 | f.publish(f.repo) | ||
| 964 | work := workRepo(t, map[string]string{"readme.md": "hi\n"}) | ||
| 965 | if _, errs, err := f.git(work, "push", f.sshURL(f.repo), "main:refs/heads/main"); err != nil { | ||
| 966 | t.Fatalf("push main: %s", errs) | ||
| 967 | } | ||
| 968 | clone := filepath.Join(t.TempDir(), "clone") | ||
| 969 | if _, errs, err := f.git("", "clone", "-q", f.sshURL(f.repo), clone); err != nil { | ||
| 970 | t.Fatalf("clone: %s", errs) | ||
| 971 | } | ||
| 972 | if _, errs, err := runGit(t, clone, "config", "user.name", "Keenan Nemetz"); err != nil { | ||
| 973 | t.Fatalf("config: %s", errs) | ||
| 862 | 974 | } | |
| 863 | 975 | ||
| 864 | out, errs, err := run("request", "new", "--title", "Roster over SSH", "-m", "body", "--author", "keenan") | ||
| 976 | out, errs, err := f.cli(clone, "request", "new", "--title", "author from the key", "-m", "body") | ||
| 865 | 977 | if err != nil { | |
| 866 | 978 | t.Fatalf("request new: %v\n%s", err, errs) | |
| 867 | 979 | } | |
| @@ -869,21 +981,74 @@ func TestRequestCLIResolvesNamesOverSSH(t *testing.T) { | |||
| 869 | 981 | if len(fields) == 0 { | |
| 870 | 982 | t.Fatalf("no request id printed: %q", out) | |
| 871 | 983 | } | |
| 872 | id := fields[0] | ||
| 873 | |||
| 874 | // What landed on the server names the account id, so the name was resolved | ||
| 875 | // from the roster rather than passed through. | ||
| 876 | 984 | raw, _, err := runGit(t, filepath.Join(f.cfg.ReposDir(), f.repo+".git"), | |
| 877 | "show", "refs/heads/vole:requests/"+id+"/request.md") | ||
| 985 | "show", "refs/heads/vole:requests/"+fields[0]+"/request.md") | ||
| 878 | 986 | if err != nil { | |
| 879 | 987 | t.Fatalf("read request from the server: %v\n%s", err, raw) | |
| 880 | 988 | } | |
| 881 | if !strings.Contains(raw, `author: keenan`) { | ||
| 882 | t.Fatalf("request lost the author name:\n%s", raw) | ||
| 989 | if !strings.Contains(raw, "author: keenan\n") { | ||
| 990 | t.Fatalf("request author is not the account:\n%s", raw) | ||
| 883 | 991 | } | |
| 884 | if !strings.Contains(raw, `author_id: `+f.ownerID) { | ||
| 992 | if !strings.Contains(raw, "author_id: "+f.ownerID) { | ||
| 885 | 993 | t.Fatalf("request author_id is not the account id %s:\n%s", f.ownerID, raw) | |
| 886 | 994 | } | |
| 995 | if strings.Contains(raw, "keenan-nemetz") { | ||
| 996 | t.Fatalf("the git-config guess reached the request:\n%s", raw) | ||
| 997 | } | ||
| 998 | if _, errs, err := f.cli(clone, "request", "new", "--title", "x", "--author", "wilbur"); err == nil { | ||
| 999 | t.Fatalf("--author is still accepted: %s", errs) | ||
| 1000 | } | ||
| 1001 | } | ||
| 1002 | |||
| 1003 | // TestAURLRemoteIsFetchedBeforeWriting: `git init` plus `--remote ssh://…` is how | ||
| 1004 | // somebody reports a bug without cloning. It used to work once per repository and | ||
| 1005 | // then fail — the fetch was skipped for a URL, so the second writer pushed from a | ||
| 1006 | // tip it had never updated, and the retry reported a "lost race" that never | ||
| 1007 | // happened and re-applied onto the same stale base. | ||
| 1008 | func TestAURLRemoteIsFetchedBeforeWriting(t *testing.T) { | ||
| 1009 | f := newFixture(t) | ||
| 1010 | f.publish(f.repo) | ||
| 1011 | fresh := func() string { | ||
| 1012 | dir := t.TempDir() | ||
| 1013 | if _, errs, err := runGit(t, dir, "init", "-q", "-b", "main"); err != nil { | ||
| 1014 | t.Fatalf("init: %v: %s", err, errs) | ||
| 1015 | } | ||
| 1016 | return dir | ||
| 1017 | } | ||
| 1018 | url := f.sshURL(f.repo) | ||
| 1019 | for i, dir := range []string{fresh(), fresh()} { | ||
| 1020 | if _, errs, err := f.cli(dir, "issue", "new", "--title", fmt.Sprintf("from repo %d", i), "--remote", url); err != nil { | ||
| 1021 | t.Fatalf("repo %d could not write through a URL: %v\n%s", i, err, errs) | ||
| 1022 | } | ||
| 1023 | } | ||
| 1024 | // The second write only lands if it fetched first, so both being on the branch | ||
| 1025 | // is the assertion. | ||
| 1026 | _, body := f.get("/r/" + f.repo + "/issues") | ||
| 1027 | for _, want := range []string{"from repo 0", "from repo 1"} { | ||
| 1028 | if !strings.Contains(body, want) { | ||
| 1029 | t.Fatalf("%q is missing from the branch:\n%s", want, body) | ||
| 1030 | } | ||
| 1031 | } | ||
| 1032 | } | ||
| 1033 | |||
| 1034 | // TestEmptyStatesPrintAWorkingCommand: every empty page is somebody's first | ||
| 1035 | // minute with the instance, and four of the five things they might want to do are | ||
| 1036 | // writes the web cannot take. The page has to hand them the command. | ||
| 1037 | func TestEmptyStatesPrintAWorkingCommand(t *testing.T) { | ||
| 1038 | f := newFixture(t) | ||
| 1039 | f.publish(f.repo) | ||
| 1040 | for _, c := range []struct{ path, want string }{ | ||
| 1041 | {"/r/" + f.repo + "/issues", "vole issue new --title"}, | ||
| 1042 | {"/r/" + f.repo + "/requests", "vole request new --title"}, | ||
| 1043 | } { | ||
| 1044 | code, body := f.get(c.path) | ||
| 1045 | if code != 200 { | ||
| 1046 | t.Fatalf("%s: %d", c.path, code) | ||
| 1047 | } | ||
| 1048 | if !strings.Contains(body, c.want) { | ||
| 1049 | t.Fatalf("%s does not print %q:\n%s", c.path, c.want, body) | ||
| 1050 | } | ||
| 1051 | } | ||
| 887 | 1052 | } | |
| 888 | 1053 | ||
| 889 | 1054 | func (f *fixture) serverIssues(t *testing.T) string { | |