From 1977786ab47801d5499a97c4695c1043d81e0501 Mon Sep 17 00:00:00 2001 From: nfebe Date: Sat, 26 Sep 2026 19:17:29 +0100 Subject: [PATCH 1/6] feat: Say when each repository was last read A list of folders said nothing about whether any of them had been read, so a graph that answered about last month looked the same as one that answered about this morning. Each row now says when it was last read, or that it is being read now. --- internal/agent/client.go | 9 +++- internal/command/root.go | 20 +++++++- internal/command/root_test.go | 47 +++++++++++++++++++ .../testdata/repositories-reading.json | 1 + internal/command/testdata/repositories.json | 2 +- internal/presentation/presentation.go | 17 +++++++ 6 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 internal/command/testdata/repositories-reading.json 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/command/root.go b/internal/command/root.go index 570294b..3e1cb45 100644 --- a/internal/command/root.go +++ b/internal/command/root.go @@ -145,14 +145,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..dff891e 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("[]")}, 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/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" + } +} From d59009aa4a50f63bceaff5fcedb34d1cc9ee7cf8 Mon Sep 17 00:00:00 2001 From: nfebe Date: Sat, 26 Sep 2026 17:54:03 +0100 Subject: [PATCH 2/6] feat: Add a command to review a checkout Reading work in progress was only reachable through the agent's page or an MCP client. It is now a command: anyone with a terminal can read what their checkout has that its default branch does not, committed or not, and gets a link to the answer. Every run names the branch and the base it compared, because a stale origin/HEAD otherwise picks the wrong base invisibly. The exit code is 2 when a skill blocks the change, so a shell script can use it. --- CHANGELOG.md | 6 + README.md | 15 +- internal/agent/reviews.go | 195 ++++++++++++ internal/command/review.go | 301 ++++++++++++++++++ internal/command/review_test.go | 241 ++++++++++++++ internal/command/root.go | 5 + internal/command/testdata/review-blocked.json | 174 ++++++++++ internal/command/testdata/review-done.json | 88 +++++ internal/command/testdata/review-failed.json | 38 +++ internal/command/testdata/review-started.json | 38 +++ 10 files changed, 1100 insertions(+), 1 deletion(-) create mode 100644 internal/agent/reviews.go create mode 100644 internal/command/review.go create mode 100644 internal/command/review_test.go create mode 100644 internal/command/testdata/review-blocked.json create mode 100644 internal/command/testdata/review-done.json create mode 100644 internal/command/testdata/review-failed.json create mode 100644 internal/command/testdata/review-started.json diff --git a/CHANGELOG.md b/CHANGELOG.md index ecaf935..03a5bbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### 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. + ## [1.0.0-beta.3] ### Added diff --git a/README.md b/README.md index 862f1fd..a26c35a 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,18 @@ # 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 @@ -74,6 +84,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 +93,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/internal/agent/reviews.go b/internal/agent/reviews.go new file mode 100644 index 0000000..b39aeb3 --- /dev/null +++ b/internal/agent/reviews.go @@ -0,0 +1,195 @@ +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. +// +// Every field the agent answers with is named here. --json re-encodes this, so +// anything missing is dropped in silence. +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. +// +// The answer comes back before the reading is done, so the caller has an id to +// come back with. +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..793820e --- /dev/null +++ b/internal/command/review.go @@ -0,0 +1,301 @@ +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. +// A review with a model takes about a minute, and the core gives up on one +// after thirty. +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, + Title: title, + 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. A review is asked for by the +// name the machine indexed, and people are standing in a directory. +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. The link above it has the rest. +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..735591b --- /dev/null +++ b/internal/command/review_test.go @@ -0,0 +1,241 @@ +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 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 3e1cb45..71524c2 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 } 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" +} From 389f13029a1395d8867147a005468f150985a6be Mon Sep 17 00:00:00 2001 From: nfebe Date: Sat, 26 Sep 2026 17:54:36 +0100 Subject: [PATCH 3/6] fix: Name a command that is on the path when no agent answers An install puts the agent somewhere the shell cannot find it, so the line printed when nothing answers told people to run something that is not there. --- internal/command/root.go | 2 +- internal/command/root_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/command/root.go b/internal/command/root.go index 71524c2..f844cbd 100644 --- a/internal/command/root.go +++ b/internal/command/root.go @@ -78,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() } diff --git a/internal/command/root_test.go b/internal/command/root_test.go index dff891e..85aa006 100644 --- a/internal/command/root_test.go +++ b/internal/command/root_test.go @@ -202,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()) } } From 277722f0e2eb0cd375a48e9051e0ee31dbbf2c3e Mon Sep 17 00:00:00 2001 From: nfebe Date: Sat, 26 Sep 2026 22:21:56 +0100 Subject: [PATCH 4/6] fix: Name a review asked for from the terminal A review with no title is a row in a list that nobody can account for. One asked for from the terminal now says so. --- internal/command/review.go | 8 +++++--- internal/command/review_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/internal/command/review.go b/internal/command/review.go index 793820e..9a1a429 100644 --- a/internal/command/review.go +++ b/internal/command/review.go @@ -54,9 +54,11 @@ func reviewCommand(opts *options) *cobra.Command { started, err := client.Review(cmd.Context(), agent.Ask{ Repository: repository, Against: against, - Title: title, - Skills: skills, - UseModel: !noModel, + // Named, so a list of reviews says where each came from. A row + // with no title is a review nobody can account for. + Title: or(title, "From the terminal"), + Skills: skills, + UseModel: !noModel, }) if err != nil { return err diff --git a/internal/command/review_test.go b/internal/command/review_test.go index 735591b..a374888 100644 --- a/internal/command/review_test.go +++ b/internal/command/review_test.go @@ -187,6 +187,30 @@ func TestWhatToCompareAgainstReachesTheAgent(t *testing.T) { } } +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("[]")}}, From 0538d13d39d70793b3488788c2cba68a3fa7a5e1 Mon Sep 17 00:00:00 2001 From: nfebe Date: Sun, 27 Sep 2026 10:58:45 +0100 Subject: [PATCH 5/6] docs(release): Prepare 1.0.0-beta.4 --- CHANGELOG.md | 16 +++++++++++++--- README.md | 5 +++-- VERSION | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03a5bbf..8258bd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,21 @@ 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. +- `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] diff --git a/README.md b/README.md index a26c35a..5e85ff6 100644 --- a/README.md +++ b/README.md @@ -14,8 +14,9 @@ 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 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 From 2eae78e8ae407df7654cefb5822bece4e30c00af Mon Sep 17 00:00:00 2001 From: nfebe Date: Mon, 28 Sep 2026 05:52:17 +0100 Subject: [PATCH 6/6] refactor: Cut comments back to their first sentence --- internal/agent/reviews.go | 9 ++------- internal/command/review.go | 10 +++------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/internal/agent/reviews.go b/internal/agent/reviews.go index b39aeb3..24b940a 100644 --- a/internal/agent/reviews.go +++ b/internal/agent/reviews.go @@ -109,9 +109,6 @@ type Read struct { } // Review is whether a checkout's work is ready to be proposed to anyone. -// -// Every field the agent answers with is named here. --json re-encodes this, so -// anything missing is dropped in silence. type Review struct { Ready bool `json:"ready"` Note string `json:"note"` @@ -146,10 +143,8 @@ const ( Failed = "failed" ) -// Review asks for a review and answers with where to find it. -// -// The answer comes back before the reading is done, so the caller has an id to -// come back with. +// 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{} diff --git a/internal/command/review.go b/internal/command/review.go index 9a1a429..a40cb7e 100644 --- a/internal/command/review.go +++ b/internal/command/review.go @@ -16,8 +16,6 @@ import ( ) // How often to ask whether a review has finished, and how long to keep asking. -// A review with a model takes about a minute, and the core gives up on one -// after thirty. var ( beat = 2 * time.Second patience = 10 * time.Minute @@ -54,8 +52,7 @@ func reviewCommand(opts *options) *cobra.Command { started, err := client.Review(cmd.Context(), agent.Ask{ Repository: repository, Against: against, - // Named, so a list of reviews says where each came from. A row - // with no title is a review nobody can account for. + // Named, so a list of reviews says where each came from. Title: or(title, "From the terminal"), Skills: skills, UseModel: !noModel, @@ -120,8 +117,7 @@ func refusal(found agent.Reading) string { return "the review failed and said nothing about why" } -// indexed is the repository a folder belongs to. A review is asked for by the -// name the machine indexed, and people are standing in a directory. +// 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 { @@ -174,7 +170,7 @@ func settled(ctx context.Context, client *agent.Client, id string) (agent.Readin } // report prints the shape of the answer: where it looked, what it made of the -// change, and every finding. The link above it has the rest. +// change, and every finding. func report(out io.Writer, found agent.Reading) { review := found.Review if found.Status == agent.Failed {