diff --git a/CHANGELOG.md b/CHANGELOG.md index ecaf935..8258bd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.0.0-beta.4] - 2026-09-27 + +### Added + +- `sourceant review` reads the work in a checkout against the branch it would be + proposed to, and prints a link to the answer. It exits 2 when a skill blocks + the change, so a shell script can use it +- `sourceant repos` says when each repository was last read, or that it is being + read now + +### Fixed + +- The line printed when no agent answers names a command that is on the path +- A review asked for from the terminal is named, so a list of reviews says where + each came from + ## [1.0.0-beta.3] ### Added diff --git a/README.md b/README.md index 862f1fd..5e85ff6 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,22 @@ # SourceAnt CLI -The command a person types. It reads the code graph the SourceAnt agent keeps on this machine. +The command a person types. It reads the work in a checkout, and the code graph the SourceAnt agent keeps on this machine. ``` +$ sourceant review +http://127.0.0.1:8930/reviews/8c2b18b67f4c41efb61efd7d2967ccfd + +feat/subtract against main (bd51156), 1 file changed, 2 commits + +blocking calc.py:5 The function 'subtract' has no docstring on its first line. +blocking calc.py:9 The function 'times' has no docstring on its first line. + +Not ready. 2 blocking. + $ sourceant repos -REPOSITORY PATH -acme/billing /home/you/work/billing +REPOSITORY READ PATH +acme/billing 3 minutes ago /home/you/work/billing +acme/shipping reading /home/you/work/shipping $ sourceant graph acme/billing 2215 nodes, 2006 links @@ -74,6 +85,7 @@ Both put the index in the same place, `$XDG_DATA_HOME/sourceant`, so it does not | Command | What it does | |---|---| +| `sourceant review [path]` | Read what a checkout has that its default branch does not | | `sourceant setup` | Put the agent and a core on this machine | | `sourceant stop` | Stop the agent and its Python core or Docker container | | `sourceant status` | Whether the agent and the indexer are running | @@ -82,6 +94,8 @@ Both put the index in the same place, `$XDG_DATA_HOME/sourceant`, so it does not | `sourceant ui` | Open the graph in a browser | | `sourceant version` | What this build is | +`review` reads the folder you are standing in, committed or not, against the branch the repository defaults to. It exits 2 when a skill blocks the change, so a shell script can use it. `--against ` compares against something else, `--no-model` says what changed without judging it, `--no-wait` prints the link and leaves it running, and `--title` and `--skill` name the change and the skills to read it against. + `--json` prints the agent's own answer, for anything that wants to read it rather than look at it. ## Building diff --git a/VERSION b/VERSION index b0a2ffd..1bdfe3e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.0.0-beta.3 +1.0.0-beta.4 diff --git a/internal/agent/client.go b/internal/agent/client.go index 920906e..3da605a 100644 --- a/internal/agent/client.go +++ b/internal/agent/client.go @@ -29,9 +29,14 @@ type Status struct { } // Repository is one repository indexed on this machine. +// +// IndexedAt is empty until it has been read, which is not the same as nothing +// having changed since, and Reading says a read is under way now. type Repository struct { - Name string `json:"name"` - Path string `json:"path"` + Name string `json:"name"` + Path string `json:"path"` + IndexedAt string `json:"indexed_at"` + Reading bool `json:"reading"` } // Node is one file, import or symbol. diff --git a/internal/agent/reviews.go b/internal/agent/reviews.go new file mode 100644 index 0000000..24b940a --- /dev/null +++ b/internal/agent/reviews.go @@ -0,0 +1,190 @@ +package agent + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" +) + +// Ask is what to review and how. +type Ask struct { + Repository string `json:"repository"` + Against string `json:"against"` + Title string `json:"title"` + Description string `json:"description"` + Skills []string `json:"skills"` + UseModel bool `json:"use_model"` +} + +// Finding is one thing a skill says is wrong with a change. +type Finding struct { + Detail string `json:"detail"` + Severity string `json:"severity"` + Path string `json:"path"` + Line *int `json:"line"` +} + +// Verdict is what one skill made of a change. +type Verdict struct { + Skill string `json:"skill"` + Passed bool `json:"passed"` + Note string `json:"note"` + Findings []Finding `json:"findings"` +} + +// ChangedFile is one file the work touches, and what changed in it. +type ChangedFile struct { + Path string `json:"path"` + Change string `json:"change"` + Patch string `json:"patch"` +} + +// Commit is one commit the branch has that its base does not. +type Commit struct { + SHA string `json:"sha"` + Author string `json:"author"` + At string `json:"at"` + Subject string `json:"subject"` + Body string `json:"body"` +} + +// Skill is one rule the review read the change against. +type Skill struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Origin string `json:"origin"` + Path string `json:"path"` + Paths []string `json:"paths"` + Reviews *bool `json:"reviews"` + Automatic bool `json:"automatic"` +} + +// Recorded is one thing known about the repository being reviewed. +type Recorded struct { + ID string `json:"id"` + Kind string `json:"kind"` + Summary string `json:"summary"` +} + +// Where is the checkout the review read, and what it was compared against. +type Where struct { + Path string `json:"path"` + Branch string `json:"branch"` + Against string `json:"against"` + Base string `json:"base"` + Commits int `json:"commits"` +} + +// Suggestion is one thing to change, and the code to put there. +type Suggestion struct { + Path string `json:"path"` + StartLine int `json:"start_line"` + EndLine int `json:"end_line"` + Side string `json:"side"` + Comment string `json:"comment"` + Category string `json:"category"` + ExistingCode string `json:"existing_code"` + SuggestedCode string `json:"suggested_code"` +} + +// Summary is the review in the order a person reads it. +type Summary struct { + Overview string `json:"overview"` + KeyImprovements []string `json:"key_improvements"` + MinorSuggestions []string `json:"minor_suggestions"` + CriticalIssues []string `json:"critical_issues"` +} + +// Read is the review proper, from the same generator a pull request gets. +type Read struct { + Verdict string `json:"verdict"` + Summary Summary `json:"summary"` + Suggestions []Suggestion `json:"suggestions"` + Notes map[string]string `json:"notes"` +} + +// Review is whether a checkout's work is ready to be proposed to anyone. +type Review struct { + Ready bool `json:"ready"` + Note string `json:"note"` + Base string `json:"base"` + Where Where `json:"where"` + Changed []ChangedFile `json:"changed"` + Commits []Commit `json:"commits"` + Skills []Skill `json:"skills"` + Knowledge []Recorded `json:"knowledge"` + Verdicts []Verdict `json:"verdicts"` + Read Read `json:"review"` +} + +// Reading is one review, whether it has finished or not. +type Reading struct { + ID string `json:"id"` + Repository string `json:"repository"` + Status string `json:"status"` + Title string `json:"title"` + Error string `json:"error"` + Started string `json:"started"` + Finished string `json:"finished"` + Review Review `json:"review"` + // Where to send somebody who wants to read it. + Path string `json:"path"` +} + +// What a review's status can be. +const ( + Running = "running" + Done = "done" + Failed = "failed" +) + +// Review asks for a review and answers with where to find it, before the +// reading is done. +func (c *Client) Review(ctx context.Context, ask Ask) (Reading, error) { + if ask.Skills == nil { + ask.Skills = []string{} + } + return post[Reading](ctx, c, "/api/reviews", ask) +} + +// Reviewed is one review by name, however long ago it ran. +func (c *Client) Reviewed(ctx context.Context, id string) (Reading, error) { + return get[Reading](ctx, c, "/api/reviews/"+url.PathEscape(id), nil) +} + +func post[T any](ctx context.Context, c *Client, path string, body any) (T, error) { + var zero T + encoded, err := json.Marshal(body) + if err != nil { + return zero, err + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+path, bytes.NewReader(encoded)) + if err != nil { + return zero, err + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + + resp, err := c.http.Do(req) + if err != nil { + return zero, &Unreachable{BaseURL: c.baseURL, Cause: err} + } + defer func() { _ = resp.Body.Close() }() + + answer, err := io.ReadAll(resp.Body) + if err != nil { + return zero, err + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return zero, &Error{StatusCode: resp.StatusCode, Detail: detail(answer)} + } + if err := json.Unmarshal(answer, &zero); err != nil { + return zero, fmt.Errorf("the agent answered %s with something other than JSON: %w", path, err) + } + return zero, nil +} diff --git a/internal/command/review.go b/internal/command/review.go new file mode 100644 index 0000000..a40cb7e --- /dev/null +++ b/internal/command/review.go @@ -0,0 +1,299 @@ +package command + +import ( + "context" + "fmt" + "io" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/sourceant/cli/internal/agent" + "github.com/sourceant/cli/internal/presentation" + "github.com/spf13/cobra" +) + +// How often to ask whether a review has finished, and how long to keep asking. +var ( + beat = 2 * time.Second + patience = 10 * time.Minute +) + +func reviewCommand(opts *options) *cobra.Command { + var ( + against string + title string + skills []string + noWait bool + noModel bool + ) + command := &cobra.Command{ + Use: "review [path]", + Short: "Read the work in a checkout before anyone else has to", + Long: "Read what a checkout has that its default branch does not, whether " + + "it is committed or not, and say whether it is ready to propose.", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + folder := "." + if len(args) == 1 { + folder = args[0] + } + folder, err := filepath.Abs(folder) + if err != nil { + return err + } + client := opts.client() + repository, err := indexed(cmd.Context(), client, folder) + if err != nil { + return err + } + started, err := client.Review(cmd.Context(), agent.Ask{ + Repository: repository, + Against: against, + // Named, so a list of reviews says where each came from. + Title: or(title, "From the terminal"), + Skills: skills, + UseModel: !noModel, + }) + if err != nil { + return err + } + link := opts.agentURL + started.Path + + if noWait { + if opts.asJSON { + return writeJSON(cmd.OutOrStdout(), started) + } + _, _ = fmt.Fprintln(cmd.OutOrStdout(), link) + return nil + } + + // Before waiting, so a long review can be opened while it runs. + if !opts.asJSON { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), link) + } + found, err := settled(cmd.Context(), client, started.ID) + if err != nil { + return err + } + if opts.asJSON { + if err := writeJSON(cmd.OutOrStdout(), found); err != nil { + return err + } + } else { + report(cmd.OutOrStdout(), found) + } + if found.Status == agent.Failed { + return fmt.Errorf("%s", refusal(found)) + } + if !found.Review.Ready { + return &unready{} + } + return nil + }, + } + command.Flags().StringVar(&against, "against", "", "Compare against this instead of the default branch") + command.Flags().StringVar(&title, "title", "", "What this change is called") + command.Flags().StringArrayVar(&skills, "skill", nil, "Read it against this skill, repeatable") + command.Flags().BoolVar(&noWait, "no-wait", false, "Print the link and leave it running") + command.Flags().BoolVar(&noModel, "no-model", false, "Say what changed without judging it") + return command +} + +// unready says the review found something blocking, which is an answer rather +// than a failure, so it exits non-zero without a second line about it. +type unready struct{} + +func (e *unready) Error() string { return "" } + +func (e *unready) code() int { return 2 } + +func refusal(found agent.Reading) string { + if found.Error != "" { + return found.Error + } + return "the review failed and said nothing about why" +} + +// indexed is the repository a folder belongs to. +func indexed(ctx context.Context, client *agent.Client, folder string) (string, error) { + repositories, err := client.Repositories(ctx) + if err != nil { + return "", err + } + folder = resolved(folder) + name, held := "", "" + for _, repository := range repositories { + path := resolved(repository.Path) + if path != folder && !strings.HasPrefix(folder, path+string(os.PathSeparator)) { + continue + } + // The innermost wins, for a checkout indexed inside another one. + if len(path) > len(held) { + name, held = repository.Name, path + } + } + if name == "" { + return "", fmt.Errorf("%s is not indexed on this machine. Add it with: sourceant repo add %s", folder, folder) + } + return name, nil +} + +func resolved(path string) string { + if real, err := filepath.EvalSymlinks(path); err == nil { + return filepath.Clean(real) + } + return filepath.Clean(path) +} + +func settled(ctx context.Context, client *agent.Client, id string) (agent.Reading, error) { + giveUp := time.Now().Add(patience) + for { + found, err := client.Reviewed(ctx, id) + if err != nil { + return found, err + } + if found.Status != agent.Running { + return found, nil + } + if time.Now().After(giveUp) { + return found, fmt.Errorf("this review is still running after %s. It carries on without us; the link has it", patience) + } + select { + case <-ctx.Done(): + return found, ctx.Err() + case <-time.After(beat): + } + } +} + +// report prints the shape of the answer: where it looked, what it made of the +// change, and every finding. +func report(out io.Writer, found agent.Reading) { + review := found.Review + if found.Status == agent.Failed { + return + } + + _, _ = fmt.Fprintf(out, "\n%s\n", locate(review)) + if review.Note != "" { + _, _ = fmt.Fprintln(out, review.Note) + } + if overview := review.Read.Summary.Overview; overview != "" { + _, _ = fmt.Fprintf(out, "\n%s\n", overview) + } + + findings := listed(review.Verdicts) + if len(findings) > 0 { + _, _ = fmt.Fprintln(out) + rows := make([][]string, 0, len(findings)) + for _, finding := range findings { + rows = append(rows, []string{finding.Severity, at(finding), finding.Detail}) + } + presentation.Table(out, nil, rows) + } + + _, _ = fmt.Fprintf(out, "\n%s\n", verdict(review, findings)) +} + +// locate says which branch was read and what it was read against, because +// origin/HEAD goes stale and a wrong base is otherwise invisible. +func locate(review agent.Review) string { + where := review.Where + against := where.Against + if against == "" { + against = "the default branch" + } + if where.Base != "" { + against = fmt.Sprintf("%s (%s)", against, short(where.Base)) + } + parts := []string{fmt.Sprintf("%s against %s", or(where.Branch, "this checkout"), against)} + if len(review.Changed) > 0 { + parts = append(parts, presentation.Count(len(review.Changed), "file", "files")+" changed") + } + if where.Commits > 0 { + parts = append(parts, presentation.Count(where.Commits, "commit", "commits")) + } + return strings.Join(parts, ", ") +} + +func verdict(review agent.Review, findings []agent.Finding) string { + blocking := 0 + for _, finding := range findings { + if finding.Severity == "blocking" { + blocking++ + } + } + counts := []string{} + if blocking > 0 { + counts = append(counts, fmt.Sprintf("%d blocking", blocking)) + } + if advisory := len(findings) - blocking; advisory > 0 { + counts = append(counts, fmt.Sprintf("%d advisory", advisory)) + } + if suggestions := len(review.Read.Suggestions); suggestions > 0 { + counts = append(counts, presentation.Count(suggestions, "suggestion", "suggestions")) + } + + answer := "Ready." + if !review.Ready { + answer = "Not ready." + } + if len(counts) == 0 { + return answer + } + return answer + " " + strings.Join(counts, ", ") + "." +} + +// listed is every finding across the skills, blocking first, then in the order +// somebody would read the files. +func listed(verdicts []agent.Verdict) []agent.Finding { + findings := []agent.Finding{} + for _, one := range verdicts { + findings = append(findings, one.Findings...) + } + sort.SliceStable(findings, func(i, j int) bool { + left, right := findings[i], findings[j] + if (left.Severity == "blocking") != (right.Severity == "blocking") { + return left.Severity == "blocking" + } + if left.Path != right.Path { + return left.Path < right.Path + } + return line(left) < line(right) + }) + return findings +} + +func at(finding agent.Finding) string { + if finding.Path == "" { + return "the change" + } + if finding.Line == nil { + return finding.Path + } + return fmt.Sprintf("%s:%d", finding.Path, *finding.Line) +} + +func line(finding agent.Finding) int { + if finding.Line == nil { + return 0 + } + return *finding.Line +} + +func short(sha string) string { + if len(sha) > 7 { + return sha[:7] + } + return sha +} + +func or(value, fallback string) string { + if value == "" { + return fallback + } + return value +} diff --git a/internal/command/review_test.go b/internal/command/review_test.go new file mode 100644 index 0000000..a374888 --- /dev/null +++ b/internal/command/review_test.go @@ -0,0 +1,265 @@ +package command + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +// indexing is the captured answer with the folder the test is standing in, so +// the review is asked for the repository that folder belongs to. +func indexing(t *testing.T, folder string) []byte { + t.Helper() + return bytes.Replace(fixture(t, "repositories.json"), []byte("/app"), []byte(folder), 1) +} + +// reviewing starts a stand-in agent that answers each path in turn, so a review +// can be running on one call and finished on the next. +func reviewing(t *testing.T, answers map[string][]answer) (func(args ...string) (string, string, int), *[]string, *[]byte) { + t.Helper() + asked := []string{} + var sent []byte + mux := http.NewServeMux() + for path, replies := range answers { + replies := replies + turn := 0 + mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { + asked = append(asked, r.Method+" "+r.URL.Path) + if r.Method == http.MethodPost { + sent, _ = readAll(r) + } + reply := replies[min(turn, len(replies)-1)] + turn++ + w.Header().Set("Content-Type", "application/json") + if reply.status != 0 { + w.WriteHeader(reply.status) + } + _, _ = w.Write(reply.body) + }) + } + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + + return func(args ...string) (string, string, int) { + var stdout, stderr bytes.Buffer + code := Run(append([]string{"--agent", server.URL}, args...), &stdout, &stderr) + return stdout.String(), stderr.String(), code + }, &asked, &sent +} + +func readAll(r *http.Request) ([]byte, error) { + var body bytes.Buffer + _, err := body.ReadFrom(r.Body) + return body.Bytes(), err +} + +func TestReviewPrintsTheLinkAndWhatWasMadeOfTheChange(t *testing.T) { + folder := t.TempDir() + run, _, _ := reviewing(t, map[string][]answer{ + "/api/repositories": {{body: indexing(t, folder)}}, + "/api/reviews": {{status: http.StatusAccepted, body: fixture(t, "review-started.json")}}, + "/api/reviews/": {{body: fixture(t, "review-done.json")}}, + }) + + stdout, stderr, code := run("review", folder) + + if code != 0 { + t.Fatalf("exited %d: %s", code, stderr) + } + for _, want := range []string{"/reviews/", "feat/subtract against main (bd51156)", "1 file changed", "Ready."} { + if !strings.Contains(stdout, want) { + t.Errorf("%q is missing from:\n%s", want, stdout) + } + } +} + +func TestABlockingFindingIsPrintedAndExitsNonZero(t *testing.T) { + folder := t.TempDir() + run, _, _ := reviewing(t, map[string][]answer{ + "/api/repositories": {{body: indexing(t, folder)}}, + "/api/reviews": {{status: http.StatusAccepted, body: fixture(t, "review-started.json")}}, + "/api/reviews/": {{body: fixture(t, "review-blocked.json")}}, + }) + + stdout, stderr, code := run("review", folder) + + if code != 2 { + t.Fatalf("exited %d, want 2: %s", code, stderr) + } + if strings.Contains(stderr, "sourceant:") { + t.Errorf("a review that found something is not an error: %q", stderr) + } + for _, want := range []string{"blocking", "calc.py:5", "Not ready.", "4 blocking"} { + if !strings.Contains(stdout, want) { + t.Errorf("%q is missing from:\n%s", want, stdout) + } + } +} + +func TestReviewKeepsAskingUntilItIsFinished(t *testing.T) { + was := beat + beat = time.Millisecond + t.Cleanup(func() { beat = was }) + + folder := t.TempDir() + run, asked, _ := reviewing(t, map[string][]answer{ + "/api/repositories": {{body: indexing(t, folder)}}, + "/api/reviews": {{status: http.StatusAccepted, body: fixture(t, "review-started.json")}}, + "/api/reviews/": { + {body: fixture(t, "review-started.json")}, + {body: fixture(t, "review-done.json")}, + }, + }) + + stdout, stderr, code := run("review", folder) + + if code != 0 { + t.Fatalf("exited %d: %s", code, stderr) + } + reads := 0 + for _, one := range *asked { + if strings.HasPrefix(one, "GET /api/reviews/") { + reads++ + } + } + if reads != 2 { + t.Errorf("read the review %d times, want it asked again while it ran", reads) + } + if !strings.Contains(stdout, "Ready.") { + t.Errorf("the finished review is missing from:\n%s", stdout) + } +} + +func TestNoWaitPrintsTheLinkAndLeavesItRunning(t *testing.T) { + folder := t.TempDir() + run, asked, _ := reviewing(t, map[string][]answer{ + "/api/repositories": {{body: indexing(t, folder)}}, + "/api/reviews": {{status: http.StatusAccepted, body: fixture(t, "review-started.json")}}, + "/api/reviews/": {{body: fixture(t, "review-done.json")}}, + }) + + stdout, stderr, code := run("review", folder, "--no-wait") + + if code != 0 { + t.Fatalf("exited %d: %s", code, stderr) + } + if !strings.Contains(stdout, "/reviews/4d4ca793285c467dbc406713a2601b00") { + t.Errorf("got %q, want the link to the review", stdout) + } + for _, one := range *asked { + if strings.HasPrefix(one, "GET /api/reviews/") { + t.Error("waited for a review it was told not to wait for") + } + } +} + +func TestWhatToCompareAgainstReachesTheAgent(t *testing.T) { + folder := t.TempDir() + run, _, sent := reviewing(t, map[string][]answer{ + "/api/repositories": {{body: indexing(t, folder)}}, + "/api/reviews": {{status: http.StatusAccepted, body: fixture(t, "review-started.json")}}, + "/api/reviews/": {{body: fixture(t, "review-done.json")}}, + }) + + _, stderr, code := run("review", folder, "--against", "dev", "--title", "Add subtract", "--no-model", "--no-wait") + + if code != 0 { + t.Fatalf("exited %d: %s", code, stderr) + } + var ask struct { + Repository string `json:"repository"` + Against string `json:"against"` + Title string `json:"title"` + UseModel bool `json:"use_model"` + } + if err := json.Unmarshal(*sent, &ask); err != nil { + t.Fatalf("could not read what was asked: %v", err) + } + if ask.Against != "dev" || ask.Title != "Add subtract" || ask.UseModel { + t.Errorf("asked %+v, want the flags as given", ask) + } + if ask.Repository != "local/sourceant" { + t.Errorf("asked for %q, want the repository the folder belongs to", ask.Repository) + } +} + +func TestAReviewNobodyNamedSaysItCameFromTheTerminal(t *testing.T) { + folder := t.TempDir() + run, _, sent := reviewing(t, map[string][]answer{ + "/api/repositories": {{body: indexing(t, folder)}}, + "/api/reviews": {{status: http.StatusAccepted, body: fixture(t, "review-started.json")}}, + "/api/reviews/": {{body: fixture(t, "review-done.json")}}, + }) + + _, stderr, code := run("review", folder, "--no-wait") + + if code != 0 { + t.Fatalf("exited %d: %s", code, stderr) + } + var ask struct { + Title string `json:"title"` + } + if err := json.Unmarshal(*sent, &ask); err != nil { + t.Fatalf("could not read what was asked: %v", err) + } + if ask.Title != "From the terminal" { + t.Errorf("asked with title %q, want where it came from", ask.Title) + } +} + +func TestAFolderNobodyIndexedNamesTheCommandThatAddsIt(t *testing.T) { + run, _, _ := reviewing(t, map[string][]answer{ + "/api/repositories": {{body: []byte("[]")}}, + }) + + _, stderr, code := run("review", t.TempDir()) + + if code != 1 { + t.Fatalf("exited %d, want 1", code) + } + if !strings.Contains(stderr, "sourceant repo add") { + t.Errorf("got %q, want how to index it", stderr) + } +} + +func TestAReviewThatFailedSaysWhyInItsOwnWords(t *testing.T) { + folder := t.TempDir() + run, _, _ := reviewing(t, map[string][]answer{ + "/api/repositories": {{body: indexing(t, folder)}}, + "/api/reviews": {{status: http.StatusAccepted, body: fixture(t, "review-started.json")}}, + "/api/reviews/": {{body: fixture(t, "review-failed.json")}}, + }) + + _, stderr, code := run("review", folder) + + if code != 1 { + t.Fatalf("exited %d, want 1", code) + } + if !strings.Contains(stderr, "No model is configured") { + t.Errorf("got %q, want the reason the agent gave", stderr) + } +} + +func TestReviewAsJSONIsTheAgentsOwnAnswer(t *testing.T) { + folder := t.TempDir() + run, _, _ := reviewing(t, map[string][]answer{ + "/api/repositories": {{body: indexing(t, folder)}}, + "/api/reviews": {{status: http.StatusAccepted, body: fixture(t, "review-started.json")}}, + "/api/reviews/": {{body: fixture(t, "review-blocked.json")}}, + }) + + stdout, _, code := run("--json", "review", folder) + + if code != 2 { + t.Fatalf("exited %d, want 2", code) + } + for _, want := range []string{`"patch"`, `"verdicts"`, `"severity"`, `"suggestions"`} { + if !strings.Contains(stdout, want) { + t.Errorf("%s was dropped on the way through:\n%s", want, stdout) + } + } +} diff --git a/internal/command/root.go b/internal/command/root.go index 570294b..f844cbd 100644 --- a/internal/command/root.go +++ b/internal/command/root.go @@ -52,6 +52,7 @@ func Run(args []string, stdout, stderr io.Writer) int { root.PersistentFlags().BoolVar(&opts.asJSON, "json", false, "Print the agent's answer as JSON") root.AddCommand( + reviewCommand(opts), stopCommand(opts), setupCommand(), statusCommand(opts), @@ -62,6 +63,10 @@ func Run(args []string, stdout, stderr io.Writer) int { ) if err := root.Execute(); err != nil { + var refused *unready + if errors.As(err, &refused) { + return refused.code() + } _, _ = fmt.Fprintln(stderr, "sourceant:", message(err)) return 1 } @@ -73,7 +78,7 @@ func Run(args []string, stdout, stderr io.Writer) int { func message(err error) string { var unreachable *agent.Unreachable if errors.As(err, &unreachable) { - return fmt.Sprintf("no agent answering at %s. Start it with sourceant-agent", unreachable.BaseURL) + return fmt.Sprintf("no agent answering at %s. Start it with sourceant ui", unreachable.BaseURL) } return err.Error() } @@ -145,14 +150,30 @@ func reposCommand(opts *options) *cobra.Command { } rows := make([][]string, 0, len(repositories)) for _, repository := range repositories { - rows = append(rows, []string{repository.Name, repository.Path}) + rows = append(rows, []string{repository.Name, read(repository), repository.Path}) } - presentation.Table(cmd.OutOrStdout(), []string{"REPOSITORY", "PATH"}, rows) + presentation.Table(cmd.OutOrStdout(), []string{"REPOSITORY", "READ", "PATH"}, rows) return nil }, } } +// read says where a repository stands: a folder nobody has read answers about +// nothing, and one being read now answers about part of itself. +func read(repository agent.Repository) string { + if repository.Reading { + return "reading" + } + if repository.IndexedAt == "" { + return "never" + } + at, err := time.Parse(time.RFC3339, repository.IndexedAt) + if err != nil { + return repository.IndexedAt + } + return presentation.Since(at) +} + func graphCommand(opts *options) *cobra.Command { var ( pathPrefix string diff --git a/internal/command/root_test.go b/internal/command/root_test.go index ea579e6..85aa006 100644 --- a/internal/command/root_test.go +++ b/internal/command/root_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/sourceant/cli/internal/agent" ) // The fixtures are answers captured from a running agent, not written here, so @@ -82,6 +84,51 @@ func TestReposListsWhatIsIndexedHere(t *testing.T) { } } +func TestReposSaysWhenEachWasLastRead(t *testing.T) { + run := running(t, map[string]answer{ + "/api/repositories": {body: fixture(t, "repositories.json")}, + }) + + stdout, stderr, code := run("repos") + + if code != 0 { + t.Fatalf("exited %d: %s", code, stderr) + } + if !strings.Contains(stdout, "READ") || !strings.Contains(stdout, "ago") { + t.Errorf("when it was read is missing from:\n%s", stdout) + } +} + +func TestReposSaysWhichFoldersAreBeingReadNow(t *testing.T) { + run := running(t, map[string]answer{ + "/api/repositories": {body: fixture(t, "repositories-reading.json")}, + }) + + stdout, stderr, code := run("repos") + + if code != 0 { + t.Fatalf("exited %d: %s", code, stderr) + } + if !strings.Contains(stdout, "reading") { + t.Errorf("a folder being read is not said to be:\n%s", stdout) + } +} + +func TestAFolderNobodyHasReadSaysNever(t *testing.T) { + for _, one := range []struct { + repository agent.Repository + want string + }{ + {agent.Repository{}, "never"}, + {agent.Repository{Reading: true}, "reading"}, + {agent.Repository{IndexedAt: "whenever"}, "whenever"}, + } { + if got := read(one.repository); got != one.want { + t.Errorf("read(%+v) = %q, want %q", one.repository, got, one.want) + } + } +} + func TestAnEmptyMachineIsToldWhatToDoNext(t *testing.T) { run := running(t, map[string]answer{ "/api/repositories": {body: []byte("[]")}, @@ -155,7 +202,7 @@ func TestAnAgentThatIsNotRunningSaysHowToStartIt(t *testing.T) { if code != 1 { t.Fatalf("exited %d, want 1", code) } - if !strings.Contains(stderr.String(), "Start it with sourceant-agent") { + if !strings.Contains(stderr.String(), "Start it with sourceant ui") { t.Errorf("got %q, want what to do about it", stderr.String()) } } diff --git a/internal/command/testdata/repositories-reading.json b/internal/command/testdata/repositories-reading.json new file mode 100644 index 0000000..a26de77 --- /dev/null +++ b/internal/command/testdata/repositories-reading.json @@ -0,0 +1 @@ +[{"name":"local/agent","path":"/home/dev/work/agent","indexed_at":"2026-09-26T18:13:29.490092+00:00","reading":false},{"name":"local/dashboard","path":"/home/dev/work/dashboard","indexed_at":"2026-09-26T18:13:28.861094+00:00","reading":true},{"name":"local/docs","path":"/home/dev/work/docs","indexed_at":"2026-09-26T18:14:11.691067+00:00","reading":false},{"name":"local/core","path":"/home/dev/work/core","indexed_at":"2026-09-26T18:12:14.498992+00:00","reading":false},{"name":"local/memory","path":"/home/dev/work/memory","indexed_at":"","reading":true},{"name":"local/webservice","path":"/home/dev/work/webservice","indexed_at":"2026-09-26T18:14:12.636184+00:00","reading":false}] diff --git a/internal/command/testdata/repositories.json b/internal/command/testdata/repositories.json index a42af7e..700950e 100644 --- a/internal/command/testdata/repositories.json +++ b/internal/command/testdata/repositories.json @@ -1 +1 @@ -[{"name":"local/sourceant","path":"/app"}] +[{"name":"local/sourceant","path":"/app","indexed_at":"2026-09-26T18:12:14.498992+00:00","reading":false}] diff --git a/internal/command/testdata/review-blocked.json b/internal/command/testdata/review-blocked.json new file mode 100644 index 0000000..e5c2547 --- /dev/null +++ b/internal/command/testdata/review-blocked.json @@ -0,0 +1,174 @@ +{ + "id": "8c2b18b67f4c41efb61efd7d2967ccfd", + "repository": "fixture/calc", + "status": "done", + "title": "Add subtract and times", + "error": "", + "started": "2026-09-26T16:46:09.277969+00:00", + "finished": "2026-09-26T16:46:09.277957+00:00", + "review": { + "ready": false, + "note": "", + "base": "bd511566c8c51c1bcb045f9682dcb5d0cec71c5b", + "where": { + "path": "/home/dev/work/calc", + "branch": "feat/subtract", + "against": "main", + "base": "bd511566c8c51c1bcb045f9682dcb5d0cec71c5b", + "commits": 2 + }, + "changed": [ + { + "path": ".claude/skills/docstrings/SKILL.md", + "change": "added", + "patch": "diff --git a/.claude/skills/docstrings/SKILL.md b/.claude/skills/docstrings/SKILL.md\nnew file mode 100644\nindex 0000000..92a8e9c\n--- /dev/null\n+++ b/.claude/skills/docstrings/SKILL.md\n@@ -0,0 +1,8 @@\n+---\n+name: docstrings\n+description: Use when reviewing any Python change in this repository.\n+---\n+\n+Every function defined in this repository must have a docstring on its first\n+line. This is a hard requirement, not a preference: a function without one\n+blocks the change.\n" + }, + { + "path": "calc.py", + "change": "modified", + "patch": "diff --git a/calc.py b/calc.py\nindex 4693ad3..91359a3 100644\n--- a/calc.py\n+++ b/calc.py\n@@ -1,2 +1,10 @@\n def add(a, b):\n return a + b\n+\n+\n+def subtract(a, b):\n+ return a - b\n+\n+\n+def times(a, b):\n+ return a * b\n" + } + ], + "commits": [ + { + "sha": "4bec62bb24ac116eae8b7b6e3948145f071fc460", + "author": "fixture", + "at": "2026-09-26T17:45:47+01:00", + "subject": "docs: Require docstrings", + "body": "" + }, + { + "sha": "3e1af8711bdc6e65fbe17f75afd55b22c47f187f", + "author": "fixture", + "at": "2026-09-26T17:43:15+01:00", + "subject": "feat: Add subtract", + "body": "" + } + ], + "skills": [ + { + "id": "docs", + "name": "docs", + "description": "docs (living docs people share, comment on and edit; use only when the user asks for one: names a doc, document, page, memo, spec, PRD, runbook or write-up, asks for somewhere to share or keep editing something, or says yes to your doc offer; a plan, comparison, summary or notes asked in chat stays in chat (at most a one-line doc offer); a report, status update, recap or \"something I can send them\" with no form named \u2192 ask first: reply, doc or file?; tabs hold tables and live charts too; a pasted claude.ai/code/artifact/\u2026 link may be a doc: check with docs tools first; not HTML pages, apps or plain chat answers; a .docx/.pptx/.xlsx/PDF asked for by name \u2192 that format's skill): asked for one \u2192 no docs-connector instructions in context? call the docs connector's `guide` with topic.instructions first, then create the doc (headings only, no body) before any search, file read or plan, even with files attached. Documenting code means docstrings or repo docs, not a doc.", + "origin": "claude", + "path": "/home/dev/.claude/skills/docs/SKILL.md", + "paths": [], + "reviews": null, + "automatic": true + }, + { + "id": "computer-use", + "name": "computer-use", + "description": "Read this skill before the first step of any request to do something in an app on the person's own computer (Notes, Finder, System Settings, any desktop app), to look at their screen, or for \"computer use\". Computer use (desktop control) lets Claude take screenshots of the person's desktop and control it with clicks, typing and scrolling through the Claude desktop app; its tools are named mcp__computer-use__* when the session runs in the desktop app and mcp__remote-devices__computer_* when a cloud session is linked to the person's computer; before computer use is turned on for a conversation there may be no such tools, only an enable__mcp__remote-devices__computer tool, which turns it on. It covers turning it on, picking the right tool, the access flow, and the safety rules for tiered apps, links and financial actions. It is not for websites, which go through Claude in Chrome or the built-in browser and their own skills.", + "origin": "claude", + "path": "/home/dev/.claude/skills/computer-use/SKILL.md", + "paths": [], + "reviews": null, + "automatic": true + }, + { + "id": "docstrings", + "name": "docstrings", + "description": "Use when reviewing any Python change in this repository.", + "origin": "claude", + "path": "/home/dev/work/calc/.claude/skills/docstrings/SKILL.md", + "paths": [], + "reviews": null, + "automatic": true + }, + { + "id": "skill-creator", + "name": "skill-creator", + "description": "Create new skills, modify and improve existing skills, and measure skill performance. Use when users want to create a skill from scratch, edit, or optimize an existing skill, run evals to test a skill, benchmark skill performance with variance analysis, or optimize a skill's description for better triggering accuracy.", + "origin": "claude", + "path": "/home/dev/.claude/skills/skill-creator/SKILL.md", + "paths": [], + "reviews": null, + "automatic": true + }, + { + "id": "built-in-browser", + "name": "built-in-browser", + "description": "Read this skill before the first step that uses the built-in browser, the browser pane inside the Claude desktop app (also called the in-app browser, the browser pane, Claude's browser, or \"your own browser\"), whose tools are named mcp__Claude_Browser__* when the session runs in the desktop app and mcp__remote-devices__Claude_Browser__* when a cloud session is linked to the person's computer; before those tools are turned on there may be a single enable__mcp__remote-devices__Claude_Browser tool instead. It covers the pane's persistent sign-ins, tabs and preview_start, reading pages as text, site approvals, what the pane cannot open, and what to do when it cannot be reached. It is not for Claude in Chrome (mcp__claude-in-chrome__* tools), which has its own skill, and it does not decide which browser to use.", + "origin": "claude", + "path": "/home/dev/.claude/skills/built-in-browser/SKILL.md", + "paths": [], + "reviews": null, + "automatic": true + } + ], + "knowledge": [], + "verdicts": [ + { + "skill": "docs", + "passed": true, + "note": "The change touches a docstrings skill and calc.py without contradicting the docs guidance, which concerns when to create docs and does not apply here.", + "findings": [] + }, + { + "skill": "computer-use", + "passed": true, + "note": "The change adds Python functions and a docstrings skill, and it does not touch anything the computer-use guidance covers, which is about desktop control tools, access flow, link safety and financial actions.", + "findings": [] + }, + { + "skill": "docstrings", + "passed": false, + "note": "The new functions subtract and times in calc.py lack the required first-line docstrings.", + "findings": [ + { + "detail": "The function 'subtract' has no docstring on its first line. Add a docstring describing the function.", + "severity": "blocking", + "path": "calc.py", + "line": 5 + }, + { + "detail": "The function 'times' has no docstring on its first line. Add a docstring describing the function.", + "severity": "blocking", + "path": "calc.py", + "line": 9 + } + ] + }, + { + "skill": "skill-creator", + "passed": false, + "note": "The change adds two functions without docstrings, violating the skill's hard requirement that every function in the repository must have a docstring on its first line.", + "findings": [ + { + "detail": "The function `subtract` is defined without a docstring on its first line. Add a docstring as the first line of the function body.", + "severity": "blocking", + "path": "calc.py", + "line": 5 + }, + { + "detail": "The function `times` is defined without a docstring on its first line. Add a docstring as the first line of the function body.", + "severity": "blocking", + "path": "calc.py", + "line": 9 + } + ] + }, + { + "skill": "built-in-browser", + "passed": true, + "note": "The change adds Python functions in calc.py unrelated to the built-in browser, so the built-in-browser guidance does not apply.", + "findings": [] + } + ], + "review": { + "verdict": "APPROVE", + "summary": { + "overview": "Adds `subtract(a, b)` and `times(a, b)` functions to `calc.py`, alongside the existing `add`. The change is pure addition of new arithmetic helpers; no existing behavior is modified. It also introduces a `.claude/skills/docstrings/SKILL.md` document stating that every Python function in the repository must have a docstring.", + "key_improvements": [ + "New `subtract` and `times` helpers extend `calc.py` without altering existing `add` behavior." + ], + "minor_suggestions": [], + "critical_issues": [] + }, + "suggestions": [], + "notes": {} + } + }, + "path": "/reviews/8c2b18b67f4c41efb61efd7d2967ccfd" +} diff --git a/internal/command/testdata/review-done.json b/internal/command/testdata/review-done.json new file mode 100644 index 0000000..93337c6 --- /dev/null +++ b/internal/command/testdata/review-done.json @@ -0,0 +1,88 @@ +{ + "id": "ea5b2c1d19544a98ad6855da3eb09761", + "repository": "fixture/calc", + "status": "done", + "title": "Add subtract and times", + "error": "", + "started": "2026-09-26T16:43:58.425621+00:00", + "finished": "2026-09-26T16:43:58.425609+00:00", + "review": { + "ready": true, + "note": "", + "base": "bd511566c8c51c1bcb045f9682dcb5d0cec71c5b", + "where": { + "path": "/home/dev/work/calc", + "branch": "feat/subtract", + "against": "main", + "base": "bd511566c8c51c1bcb045f9682dcb5d0cec71c5b", + "commits": 1 + }, + "changed": [ + { + "path": "calc.py", + "change": "modified", + "patch": "diff --git a/calc.py b/calc.py\nindex 4693ad3..91359a3 100644\n--- a/calc.py\n+++ b/calc.py\n@@ -1,2 +1,10 @@\n def add(a, b):\n return a + b\n+\n+\n+def subtract(a, b):\n+ return a - b\n+\n+\n+def times(a, b):\n+ return a * b\n" + } + ], + "commits": [ + { + "sha": "3e1af8711bdc6e65fbe17f75afd55b22c47f187f", + "author": "fixture", + "at": "2026-09-26T17:43:15+01:00", + "subject": "feat: Add subtract", + "body": "" + } + ], + "skills": [ + { + "id": "pptx", + "name": "pptx", + "description": "Use this skill any time a .pptx or .potx file is involved in any way \u2014 as input, output, or both. This includes: creating slide decks, pitch decks, or presentations as PowerPoint (.pptx) files; reading, parsing, or extracting text from any .pptx or .potx file (even if the extracted content will be used elsewhere, like in an email, summary, or creating a different type of slide deck); editing, modifying, or updating existing presentations; combining or splitting slide files; working with templates (.potx), layouts, speaker notes, or comments. Trigger whenever the user asks for a PowerPoint or .pptx file, or references a .pptx or .potx filename, regardless of what they plan to do with the content afterward. However, when the user asks for a deck, slides, a slide deck, or a presentation without naming a file format, default to using a dedicated slide-deck artifact type or a separate slides skill if this session offers one; otherwise, use this skill.", + "origin": "claude", + "path": "/home/dev/.claude/skills/pptx/SKILL.md", + "paths": [], + "reviews": null, + "automatic": true + }, + { + "id": "xlsx", + "name": "xlsx", + "description": "Use this skill any time a spreadsheet file is the primary input or output. This means any task where the user wants to: open, read, edit, or fix an existing .xlsx, .xlsm, .xltx, .csv, or .tsv file (e.g., adding columns, computing formulas, formatting, charting, cleaning messy data); create a new spreadsheet from scratch or from other data sources; or convert between tabular file formats. Trigger especially when the user references a spreadsheet file by name or path \u2014 even casually (like \"the xlsx in my downloads\") \u2014 and wants something done to it or produced from it. Also trigger for cleaning or restructuring messy tabular data files (malformed rows, misplaced headers, junk data) into proper spreadsheets. The deliverable must be a spreadsheet file. Do NOT trigger when the primary deliverable is a Word document, HTML report, standalone Python script, database pipeline, or Google Sheets API integration, even if tabular data is involved.", + "origin": "claude", + "path": "/home/dev/.claude/skills/xlsx/SKILL.md", + "paths": [], + "reviews": null, + "automatic": true + } + ], + "knowledge": [], + "verdicts": [ + { + "skill": "pptx", + "passed": true, + "note": "The change adds pure Python functions to calc.py and involves no .pptx or .potx files, so the pptx guidance does not apply.", + "findings": [] + }, + { + "skill": "xlsx", + "passed": true, + "note": "The guidance applies only to spreadsheet-file tasks, and this change adds plain arithmetic functions to a Python file with no spreadsheet involvement.", + "findings": [] + } + ], + "review": { + "verdict": "APPROVE", + "summary": { + "overview": "Adds two new functions to calc.py: `subtract(a, b)` returning `a - b` and `times(a, b)` returning `a * b`, alongside the existing `add`. No existing behavior changes.", + "key_improvements": [ + "Adds `subtract` and `times` arithmetic helpers in calc.py." + ], + "minor_suggestions": [], + "critical_issues": [] + }, + "suggestions": [], + "notes": {} + } + }, + "path": "/reviews/ea5b2c1d19544a98ad6855da3eb09761" +} diff --git a/internal/command/testdata/review-failed.json b/internal/command/testdata/review-failed.json new file mode 100644 index 0000000..317a5b1 --- /dev/null +++ b/internal/command/testdata/review-failed.json @@ -0,0 +1,38 @@ +{ + "id": "1eda0d6fa03548969d3a43dd222c9df3", + "repository": "fixture/calc", + "status": "failed", + "title": "Add subtract and times", + "error": "No model is configured. Choose one in Settings, or ask for what changed without judging it.", + "started": "2026-09-26T16:39:38.188811+00:00", + "finished": "2026-09-26T16:39:38.188804+00:00", + "review": { + "ready": false, + "note": "", + "base": "", + "where": { + "path": "", + "branch": "", + "against": "", + "base": "", + "commits": 0 + }, + "changed": [], + "commits": null, + "skills": [], + "knowledge": [], + "verdicts": [], + "review": { + "verdict": "", + "summary": { + "overview": "", + "key_improvements": null, + "minor_suggestions": null, + "critical_issues": null + }, + "suggestions": [], + "notes": null + } + }, + "path": "/reviews/1eda0d6fa03548969d3a43dd222c9df3" +} diff --git a/internal/command/testdata/review-started.json b/internal/command/testdata/review-started.json new file mode 100644 index 0000000..d0dae81 --- /dev/null +++ b/internal/command/testdata/review-started.json @@ -0,0 +1,38 @@ +{ + "id": "4d4ca793285c467dbc406713a2601b00", + "repository": "fixture/calc", + "status": "running", + "title": "Add subtract and times", + "error": "", + "started": "2026-09-26T16:43:27.552516+00:00", + "finished": "", + "review": { + "ready": false, + "note": "", + "base": "", + "where": { + "path": "", + "branch": "", + "against": "", + "base": "", + "commits": 0 + }, + "changed": null, + "commits": null, + "skills": null, + "knowledge": null, + "verdicts": null, + "review": { + "verdict": "", + "summary": { + "overview": "", + "key_improvements": null, + "minor_suggestions": null, + "critical_issues": null + }, + "suggestions": null, + "notes": null + } + }, + "path": "/reviews/4d4ca793285c467dbc406713a2601b00" +} diff --git a/internal/presentation/presentation.go b/internal/presentation/presentation.go index b2e237d..6c94ba7 100644 --- a/internal/presentation/presentation.go +++ b/internal/presentation/presentation.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "text/tabwriter" + "time" ) // Table writes aligned columns, header first. @@ -36,3 +37,19 @@ func Count(n int, singular, plural string) string { } return fmt.Sprintf("%d %s", n, plural) } + +// Since renders how long ago a moment was, in the coarsest unit that still says +// something: a person reading a table wants "3 days", not 4,317 minutes. +func Since(at time.Time) string { + elapsed := time.Since(at) + switch { + case elapsed < time.Minute: + return "just now" + case elapsed < time.Hour: + return Count(int(elapsed.Minutes()), "minute", "minutes") + " ago" + case elapsed < 24*time.Hour: + return Count(int(elapsed.Hours()), "hour", "hours") + " ago" + default: + return Count(int(elapsed.Hours()/24), "day", "days") + " ago" + } +}