From 483153794f0e9cd079fc3ad8240282e2f18a5782 Mon Sep 17 00:00:00 2001 From: tr-heuser Date: Thu, 22 Oct 2026 16:33:59 -0700 Subject: [PATCH 1/4] add an experimental build tag Commands for internal dogfooding go behind the experimental Go build tag, so no release (stable or nightly) includes them. config.Experimental gates smaller changes inside existing commands. CI builds, tests, and smoke-tests both variants, and golangci-lint runs with the tag on. --- .github/workflows/go.yml | 6 ++++++ .golangci.yaml | 5 +++++ CLAUDE.md | 22 ++++++++++++++++++++++ internal/config/experimental.go | 7 +++++++ internal/config/experimental_off.go | 7 +++++++ justfile | 7 ++++++- 6 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 internal/config/experimental.go create mode 100644 internal/config/experimental_off.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index eb26d5d..9cfef82 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -30,6 +30,12 @@ jobs: run: go test -v --vet=all ./... - name: CLI smoke test run: go run ./cmd/aviator --help + - name: Build (experimental) + run: go build -v -tags experimental ./... + - name: Test (experimental) + run: go test -v --vet=all -tags experimental ./... + - name: CLI smoke test (experimental) + run: go run -tags experimental ./cmd/aviator --help golangci-lint: runs-on: ubuntu-latest diff --git a/.golangci.yaml b/.golangci.yaml index c0836b3..5cca199 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,4 +1,9 @@ version: "2" +run: + # Lint the files behind the experimental build tag too; the !experimental + # stubs are trivial and covered by the untagged build. + build-tags: + - experimental linters: enable: - asasalint diff --git a/CLAUDE.md b/CLAUDE.md index 11f753f..08d4c9e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -94,6 +94,28 @@ Equivalent raw commands: `go build ./...`, `go test --vet=all ./...`, no `formatters` block: golangci-lint bundles its own older copies of both, and two copies grading the same files is how CI deadlocks. +## Experimental features + +New features that are only for internal dogfooding sit behind the +`experimental` Go build tag, so they are left out of stable binaries entirely. +Nobody can switch them on with a flag or env var. + +- A new command goes in its own file that starts with `//go:build experimental` + and registers itself: `func init() { rootCmd.AddCommand(fooCmd) }`. Don't + list it in `main.go`. +- Mark a new top-level command `Hidden: true`, so even experimental builds + leave it out of `--help`, `aviator help`, and shell completion. It still runs + when called by name. +- A new flag or branch inside an existing command is gated on + `config.Experimental` (a const, so the stable build drops the dead code). +- No release (stable or nightly) is built with the tag; both are public. The + team builds its own binary with `just build-experimental` or + `go run -tags experimental ./cmd/aviator`. +- CI builds, tests, and smoke-tests both variants; golangci-lint runs with the + tag on. +- To ship a feature to everyone, delete the build tag or the `config.Experimental` + check. + ## CI and releases - `.github/workflows/go.yml` — build, test, smoke test, golangci-lint on PRs. diff --git a/internal/config/experimental.go b/internal/config/experimental.go new file mode 100644 index 0000000..3c53377 --- /dev/null +++ b/internal/config/experimental.go @@ -0,0 +1,7 @@ +//go:build experimental + +package config + +// Experimental reports whether this binary was built with the experimental +// tag. No release sets it; only local `just build-experimental` builds do. +const Experimental = true diff --git a/internal/config/experimental_off.go b/internal/config/experimental_off.go new file mode 100644 index 0000000..1fb136e --- /dev/null +++ b/internal/config/experimental_off.go @@ -0,0 +1,7 @@ +//go:build !experimental + +package config + +// Experimental reports whether this binary was built with the experimental +// tag. No release sets it; only local `just build-experimental` builds do. +const Experimental = false diff --git a/justfile b/justfile index 5b8a3ec..b1ae0a2 100644 --- a/justfile +++ b/justfile @@ -5,9 +5,14 @@ default: build: go build -o aviator ./cmd/aviator -# Run all tests with vet +# Build ./aviator with the experimental (internal-only) commands +build-experimental: + go build -tags experimental -o aviator ./cmd/aviator + +# Run all tests with vet, with and without the experimental tag test: go test --vet=all ./... + go test --vet=all -tags experimental ./... # Check formatting and run golangci-lint (golangci-lint must be installed) lint: From f119edb951a73288f40009b2b40888226fccf63d Mon Sep 17 00:00:00 2001 From: tr-heuser Date: Wed, 28 Oct 2026 12:17:43 -0700 Subject: [PATCH 2/4] add experimental decisions commands aviator decisions list, show, create, edit, comment (and comment --edit), settle, reopen, and apply record decisions on a Verify session's pull request through POST /decisions/batch. Every command sends one batch, so a change across several decisions is all or nothing; apply sends its JSON file as the batch body. Each write is recorded under a --role (person, coding_agent, review_agent, external_bot), with --model, --harness, and --skill describing the agent when there is one. Hidden from help and completion. --- CLAUDE.md | 30 ++ cmd/aviator/decisions.go | 586 +++++++++++++++++++++++++++++++ cmd/aviator/decisions_format.go | 190 ++++++++++ cmd/aviator/decisions_test.go | 187 ++++++++++ cmd/aviator/experimental_test.go | 29 ++ internal/api/decisions.go | 179 ++++++++++ internal/api/decisions_test.go | 133 +++++++ 7 files changed, 1334 insertions(+) create mode 100644 cmd/aviator/decisions.go create mode 100644 cmd/aviator/decisions_format.go create mode 100644 cmd/aviator/decisions_test.go create mode 100644 cmd/aviator/experimental_test.go create mode 100644 internal/api/decisions.go create mode 100644 internal/api/decisions_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 08d4c9e..c68df66 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,6 +29,8 @@ cmd/aviator/ # CLI entry point + commands (one file per command) evidence.go # `aviator evidence` -> signed URL / download of one evidence file invariants.go # `aviator invariants ` invariants_format.go # text rendering for the invariants commands + decisions.go # `aviator decisions ` (experimental build only) + decisions_format.go # text rendering for the decisions commands version.go # `aviator version` helpers.go # parseRepo(s), readSpecFile, collectCriteria, collectBody internal/ @@ -180,6 +182,34 @@ The CLI targets endpoints in the `mergeit` backend: The CLI doesn't follow it with the bearer token: `EvidenceURL` stops at the redirect and the download is a plain unauthenticated GET. +- `/api/v1/verify//decisions` — experimental build only. Every body + rejects unknown keys. + - `GET` — `{decisions: [...]}`, open first. `GET /` — one decision. + Reads take any API token. + - `POST /batch` — `{decisions: [{id?, author, type?, title?, summary?, + anchor?, outcome?, resolution?, comments?: [{author, body, + model_metadata?}]}]}`, all-or-nothing, decisions returned in request + order. No `id` creates (`type` and `title` required); an `id` changes only + the keys sent. `outcome` settles, `outcome: null` reopens (and clears + `resolution`), `anchor: null` clears the anchor. A settled decision + accepts only reopening and comments (409 otherwise). The same `id` can't + appear twice. The CLI models the three-state keys with `api.Nullable`. + - `author` is `{role, model?, harness?, skills?}` on every item and comment; + `role` is `person`, `coding_agent`, `review_agent`, or `external_bot`. The + CLI's `--role`, `--model`, `--harness`, and `--skill` fill it. + `model_metadata` is free-form, per comment, stored but not returned. + - No `via`: the server credits an `aviator login` token to `aviator-cli` and + a personal token to `api`. + - `PATCH //comments/` — `{body}`. + - Writes need a user token on an account with Verify. Only the user who + raised a decision can change it (403); anyone in the account can comment. + - Responses carry `raised_by` (the user who created it), `author` (of the + latest change), `resolution`, and per comment a `user` (the account) and + an `author` (the role). + - Every command sends one batch: `aviator decisions apply` sends its file + as the batch body, and the single-decision commands build one, an item per + decision ID. + `POST` and `PATCH /api/v1/verify`, dismissals, and the listing are gated on `role="user"`: an account-scoped API token resolves to no role and gets a 403, so they need a user token or an `aviator login` session. diff --git a/cmd/aviator/decisions.go b/cmd/aviator/decisions.go new file mode 100644 index 0000000..cc661ee --- /dev/null +++ b/cmd/aviator/decisions.go @@ -0,0 +1,586 @@ +//go:build experimental + +package main + +import ( + "encoding/json" + "fmt" + "io" + "os" + "strconv" + "strings" + + "emperror.dev/errors" + "github.com/aviator-co/aviator-cli/internal/api" + "github.com/aviator-co/aviator-cli/internal/utils/colors" + "github.com/dustin/go-humanize/english" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// decisionTypes, decisionOutcomes, and decisionRoles name the server's values +// for flag help. The server validates them, so nothing is checked client-side. +const ( + decisionTypes = "question, possible_bug, risk, choice" + decisionOutcomes = "fixed, intended, accepted_risk, follow_up, withdrawn, decided" + decisionRoles = "person, coding_agent, review_agent, external_bot" +) + +// decisionsFlags holds the flags every decisions subcommand shares. +var decisionsFlags struct { + JSON bool +} + +// authorFlags are the role a write is recorded as, and the agent details +// behind it when known. +type authorFlags struct { + Role string + Model string + Harness string + Skills []string +} + +func (f *authorFlags) author() api.DecisionAuthor { + return api.DecisionAuthor{Role: f.Role, Model: f.Model, Harness: f.Harness, Skills: f.Skills} +} + +var decisionsCmd = &cobra.Command{ + Use: "decisions", + Aliases: []string{"decision"}, + Short: "Read and record the decisions on a session's pull request", + // Experimental: left out of help and completion. + Hidden: true, + Long: "A decision is a question, possible bug, risk, or choice raised about a\n" + + "session's pull request. Each one has a comment thread and is either open\n" + + "or settled with an outcome.\n" + + "\n" + + "Every write is recorded under a --role (" + decisionRoles + "),\n" + + "with --model, --harness, and --skill describing the agent when there is\n" + + "one. edit, comment, settle, and reopen take several decision IDs and apply\n" + + "to all of them, and apply records a whole JSON batch. Each command sends\n" + + "one request, so either all of it is recorded or none of it is.\n" + + "\n" + + "Writes act as the caller, so they need a user access token. Comments are\n" + + "credited to aviator-cli when you're signed in with `aviator login`, and\n" + + "to the API when you use a personal token.", +} + +var decisionsListCmd = &cobra.Command{ + Use: "list ", + Short: "List a session's decisions, open ones first (e.g. aviator decisions list r/123)", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + runbookNumber, err := parseRunbookID(args[0]) + if err != nil { + return err + } + client, err := api.NewClient() + if err != nil { + return err + } + resp, err := client.ListDecisions(cmd.Context(), runbookNumber) + if err != nil { + return err + } + if decisionsFlags.JSON { + return printJSON(newDecisionListJSON(runbookNumber, resp.Decisions)) + } + if len(resp.Decisions) == 0 { + fmt.Printf("No decisions on %s.\n", formatRunbookID(runbookNumber)) + return nil + } + fmt.Print(formatDecisionTable(resp.Decisions)) + return nil + }, +} + +var decisionsShowCmd = &cobra.Command{ + Use: "show ", + Short: "Show one decision with its comment thread", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + runbookNumber, err := parseRunbookID(args[0]) + if err != nil { + return err + } + decisionID, err := parseDecisionID(args[1]) + if err != nil { + return err + } + client, err := api.NewClient() + if err != nil { + return err + } + d, err := client.GetDecision(cmd.Context(), runbookNumber, decisionID) + if err != nil { + return err + } + if decisionsFlags.JSON { + return printJSON(newDecisionJSON(d)) + } + fmt.Print(formatDecisionDetail(d)) + return nil + }, +} + +var decisionsCreateFlags struct { + Type string + Title string + Summary string + Path string + Lines string + Commit string + Body string + BodyFile string + Author authorFlags +} + +var decisionsCreateCmd = &cobra.Command{ + Use: "create ", + Short: "Raise a new decision on a session", + Long: "Raise a new decision on a session as --role. --body is the first\n" + + "comment on its thread, explaining why it was raised. --path, --lines,\n" + + "and --commit optionally pin it to lines of a file.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + f := &decisionsCreateFlags + body, err := collectBody(f.Body, f.BodyFile) + if err != nil { + return err + } + if body == "" { + return errors.New("--body (or --body-file) is required") + } + anchor, err := buildAnchor(f.Path, f.Lines, f.Commit) + if err != nil { + return err + } + author := f.Author.author() + return recordDecisions(cmd, args[0], []api.DecisionBatchItem{{ + Author: author, + Type: f.Type, + Title: f.Title, + Summary: f.Summary, + Anchor: anchor, + Comments: []api.DecisionCommentInput{{Author: author, Body: body}}, + }}) + }, +} + +var decisionsEditFlags struct { + Type string + Title string + Summary string + Path string + Lines string + Commit string + Body string + BodyFile string + Author authorFlags +} + +var decisionsEditCmd = &cobra.Command{ + Use: "edit ...", + Short: "Change open decisions; every listed one gets the same changes", + Long: "Change the type, title, summary, or anchor of open decisions as\n" + + "--role. --body also adds a comment to each one's thread. Only the user\n" + + "who raised a decision can edit it, and settled decisions can't be edited.", + Args: cobra.MinimumNArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + f := &decisionsEditFlags + body, err := collectBody(f.Body, f.BodyFile) + if err != nil { + return err + } + anchor, err := buildAnchor(f.Path, f.Lines, f.Commit) + if err != nil { + return err + } + if f.Type == "" && f.Title == "" && f.Summary == "" && anchor.IsZero() && body == "" { + return errors.New("nothing to change: pass --type, --title, --summary, an anchor, or --body") + } + author := f.Author.author() + return recordForEach(cmd, args, func(id int) api.DecisionBatchItem { + item := api.DecisionBatchItem{ + ID: id, + Author: author, + Type: f.Type, + Title: f.Title, + Summary: f.Summary, + Anchor: anchor, + } + if body != "" { + item.Comments = []api.DecisionCommentInput{{Author: author, Body: body}} + } + return item + }) + }, +} + +var decisionsCommentFlags struct { + Body string + BodyFile string + Edit int + Author authorFlags +} + +var decisionsCommentCmd = &cobra.Command{ + Use: "comment ...", + Short: "Add the same comment to decisions, or edit one with --edit", + Long: "Add the same comment to each decision's thread as --role.\n" + + "\n" + + "--edit instead replaces the body of one of your comments on\n" + + "one decision (comment IDs are in `aviator decisions show`). Only the user\n" + + "who wrote a comment can edit it, and it keeps its author, so --edit takes\n" + + "no --role.", + Args: cobra.MinimumNArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + f := &decisionsCommentFlags + body, err := collectBody(f.Body, f.BodyFile) + if err != nil { + return err + } + if body == "" { + return errors.New("--body (or --body-file) is required") + } + if cmd.Flags().Changed("edit") { + return editDecisionComment(cmd, args, f.Edit, body) + } + if f.Author.Role == "" { + return errors.New("--role is required to add a comment") + } + author := f.Author.author() + return recordForEach(cmd, args, func(id int) api.DecisionBatchItem { + return api.DecisionBatchItem{ + ID: id, + Author: author, + Comments: []api.DecisionCommentInput{{Author: author, Body: body}}, + } + }) + }, +} + +var decisionsSettleFlags struct { + Outcome string + Summary string + Resolution string + Author authorFlags +} + +var decisionsSettleCmd = &cobra.Command{ + Use: "settle ...", + Short: "Settle open decisions with an outcome (e.g. aviator decisions settle r/123 7 8 --outcome fixed --resolution why --role person)", + Long: "Settle open decisions with an outcome (" + decisionOutcomes + ") as\n" + + "--role. --resolution says why it was settled this way, and --summary\n" + + "sets the decision's summary of what was decided in the same change. A\n" + + "settled decision can't be edited or settled again until it is reopened.", + Args: cobra.MinimumNArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + f := &decisionsSettleFlags + author := f.Author.author() + return recordForEach(cmd, args, func(id int) api.DecisionBatchItem { + return api.DecisionBatchItem{ + ID: id, + Author: author, + Summary: f.Summary, + Outcome: api.ValueOf(f.Outcome), + Resolution: strings.TrimSpace(f.Resolution), + } + }) + }, +} + +var decisionsReopenFlags struct { + Body string + BodyFile string + Author authorFlags +} + +var decisionsReopenCmd = &cobra.Command{ + Use: "reopen ...", + Short: "Reopen settled decisions", + Long: "Reopen settled decisions as --role, clearing their outcome and\n" + + "resolution. --body adds a comment saying why. Only the user who raised a\n" + + "decision can reopen it.", + Args: cobra.MinimumNArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + f := &decisionsReopenFlags + body, err := collectBody(f.Body, f.BodyFile) + if err != nil { + return err + } + author := f.Author.author() + return recordForEach(cmd, args, func(id int) api.DecisionBatchItem { + item := api.DecisionBatchItem{ID: id, Author: author, Outcome: api.Null[string]()} + if body != "" { + item.Comments = []api.DecisionCommentInput{{Author: author, Body: body}} + } + return item + }) + }, +} + +var decisionsApplyFlags struct { + File string +} + +var decisionsApplyCmd = &cobra.Command{ + Use: "apply ", + Short: "Create, edit, comment on, settle, and reopen decisions from a JSON batch file", + Long: "Record a batch of decisions from a JSON file (- reads stdin), all or\n" + + "nothing:\n" + + "\n" + + " {\"decisions\": [\n" + + " {\"author\": {\"role\": \"coding_agent\", \"model\": \"claude-fable-5-1\",\n" + + " \"harness\": \"claude-code\", \"skills\": [\"go-backend\"]},\n" + + " \"type\": \"risk\", \"title\": \"...\", \"summary\": \"...\",\n" + + " \"anchor\": {\"commit_sha\": \"abc1234\", \"file_path\": \"a.go\", \"start_line\": 10, \"end_line\": 20},\n" + + " \"comments\": [{\"author\": {\"role\": \"coding_agent\"}, \"body\": \"why it was raised\",\n" + + " \"model_metadata\": {\"subagent\": \"reviewer\"}}]},\n" + + " {\"id\": 7, \"author\": {\"role\": \"person\"},\n" + + " \"comments\": [{\"author\": {\"role\": \"person\"}, \"body\": \"a comment\"}]},\n" + + " {\"id\": 8, \"author\": {\"role\": \"person\"}, \"outcome\": \"fixed\", \"resolution\": \"why\"},\n" + + " {\"id\": 9, \"author\": {\"role\": \"person\"}, \"outcome\": null}\n" + + " ]}\n" + + "\n" + + "An item without an id creates a decision and needs a type and title. One\n" + + "with an id changes only the keys it has: an outcome settles it, a null\n" + + "outcome reopens it, a null anchor clears its anchor, and comments alone\n" + + "only add comments. Every item and comment needs an author with a role\n" + + "(" + decisionRoles + "); add model, harness, and\n" + + "skills when you know them. Unknown keys are refused before anything is\n" + + "sent.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + batch, err := readDecisionBatch(decisionsApplyFlags.File) + if err != nil { + return err + } + return recordDecisions(cmd, args[0], batch.Decisions) + }, +} + +// recordForEach builds one item per decision ID in args[1:] and records them +// as a single batch against the session in args[0]. +func recordForEach(cmd *cobra.Command, args []string, item func(id int) api.DecisionBatchItem) error { + items := make([]api.DecisionBatchItem, 0, len(args)-1) + for _, arg := range args[1:] { + id, err := parseDecisionID(arg) + if err != nil { + return err + } + items = append(items, item(id)) + } + return recordDecisions(cmd, args[0], items) +} + +// recordDecisions sends items to the session as one batch, then prints the +// decisions it touched as they now stand. +func recordDecisions(cmd *cobra.Command, sessionArg string, items []api.DecisionBatchItem) error { + runbookNumber, err := parseRunbookID(sessionArg) + if err != nil { + return err + } + client, err := api.NewClient() + if err != nil { + return err + } + resp, err := client.ApplyDecisions(cmd.Context(), runbookNumber, api.DecisionBatchRequest{Decisions: items}) + if err != nil { + return err + } + if decisionsFlags.JSON { + return printJSON(newDecisionListJSON(runbookNumber, resp.Decisions)) + } + fmt.Printf("%s %s: %s recorded\n", colors.Success("✓"), + formatRunbookID(runbookNumber), english.Plural(len(resp.Decisions), "decision", "")) + fmt.Print(formatDecisionTable(resp.Decisions)) + return nil +} + +// editDecisionComment replaces the body of comment commentID on the one +// decision in args[1]. +func editDecisionComment(cmd *cobra.Command, args []string, commentID int, body string) error { + if len(args) != 2 { + return errors.New("--edit takes exactly one decision ID") + } + if commentID <= 0 { + return errors.Errorf("invalid --edit %d, expected a comment ID", commentID) + } + runbookNumber, err := parseRunbookID(args[0]) + if err != nil { + return err + } + decisionID, err := parseDecisionID(args[1]) + if err != nil { + return err + } + client, err := api.NewClient() + if err != nil { + return err + } + c, err := client.EditDecisionComment(cmd.Context(), runbookNumber, decisionID, commentID, body) + if err != nil { + return err + } + if decisionsFlags.JSON { + return printJSON(newDecisionCommentJSON(c)) + } + fmt.Printf("%s %s: comment #%d on #%d edited\n", colors.Success("✓"), + formatRunbookID(runbookNumber), c.ID, decisionID) + fmt.Println(indent(strings.TrimSpace(c.Body), " ")) + return nil +} + +// buildAnchor turns the anchor flags into an anchor, left out when none were +// passed. +func buildAnchor(path, lines, commit string) (api.Nullable[api.DecisionAnchor], error) { + if path == "" && lines == "" && commit == "" { + return api.Nullable[api.DecisionAnchor]{}, nil + } + if lines != "" && path == "" { + return api.Nullable[api.DecisionAnchor]{}, errors.New("--lines needs --path") + } + anchor := api.DecisionAnchor{FilePath: path, CommitSHA: commit} + if lines != "" { + start, end, err := parseLineRange(lines) + if err != nil { + return api.Nullable[api.DecisionAnchor]{}, err + } + anchor.StartLine, anchor.EndLine = start, end + } + return api.ValueOf(anchor), nil +} + +// parseLineRange reads --lines as or -. +func parseLineRange(s string) (start, end int, err error) { + first, last, isRange := strings.Cut(strings.TrimSpace(s), "-") + if !isRange { + last = first + } + start, errStart := strconv.Atoi(strings.TrimSpace(first)) + end, errEnd := strconv.Atoi(strings.TrimSpace(last)) + if errStart != nil || errEnd != nil || start < 1 || end < start { + return 0, 0, errors.Errorf("invalid --lines %q, expected or -", s) + } + return start, end, nil +} + +// parseDecisionID accepts a bare number or the # form the CLI prints. +func parseDecisionID(arg string) (int, error) { + s := strings.TrimPrefix(strings.TrimSpace(arg), "#") + n, err := strconv.Atoi(s) + if err != nil || n <= 0 { + return 0, errors.Errorf("invalid decision ID %q, expected a positive number", arg) + } + return n, nil +} + +// readDecisionBatch reads apply's file, which is the API's batch body. Unknown +// keys are refused, so a typo, or a via or batch-level model_metadata from the +// old format, is caught before anything is sent. +func readDecisionBatch(path string) (*api.DecisionBatchRequest, error) { + var data []byte + var err error + if path == "-" { + data, err = io.ReadAll(os.Stdin) + } else { + data, err = os.ReadFile(path) + } + if err != nil { + return nil, errors.Wrapf(err, "failed to read %s", path) + } + dec := json.NewDecoder(strings.NewReader(string(data))) + dec.DisallowUnknownFields() + var req api.DecisionBatchRequest + if err := dec.Decode(&req); err != nil { + return nil, errors.Wrapf(err, "failed to parse %s", path) + } + if len(req.Decisions) == 0 { + return nil, errors.Errorf("%s has no decisions", path) + } + return &req, nil +} + +// registerAuthorFlags registers --role, --model, --harness, and --skill; +// required marks --role required, for commands where every use writes. +func registerAuthorFlags(cmd *cobra.Command, f *authorFlags, required bool) { + fs := cmd.Flags() + fs.StringVar(&f.Role, "role", "", "record this under a role ("+decisionRoles+")") + fs.StringVar(&f.Model, "model", "", "the model behind this, for an agent") + fs.StringVar(&f.Harness, "harness", "", "the harness the agent runs in (e.g. claude-code)") + fs.StringArrayVar(&f.Skills, "skill", nil, "a skill the agent used (repeatable)") + if required { + _ = cmd.MarkFlagRequired("role") + } +} + +func registerAnchorFlags(fs *pflag.FlagSet, path, lines, commit *string) { + fs.StringVar(path, "path", "", "pin to this file path") + fs.StringVar(lines, "lines", "", "pin to or - in --path") + fs.StringVar(commit, "commit", "", "the commit SHA the anchor refers to") +} + +func init() { + decisionsCmd.PersistentFlags().BoolVar(&decisionsFlags.JSON, "json", false, + "print the result as a single JSON object instead of the human summary") + + cf := decisionsCreateCmd.Flags() + cf.StringVar(&decisionsCreateFlags.Type, "type", "", "the kind of decision ("+decisionTypes+")") + cf.StringVar(&decisionsCreateFlags.Title, "title", "", "a one-line title") + cf.StringVar(&decisionsCreateFlags.Summary, "summary", "", "an optional longer description") + registerAnchorFlags(cf, &decisionsCreateFlags.Path, &decisionsCreateFlags.Lines, &decisionsCreateFlags.Commit) + registerBodyFlags(cf, &decisionsCreateFlags.Body, &decisionsCreateFlags.BodyFile, + "the first comment, explaining the decision") + registerAuthorFlags(decisionsCreateCmd, &decisionsCreateFlags.Author, true) + _ = decisionsCreateCmd.MarkFlagRequired("type") + _ = decisionsCreateCmd.MarkFlagRequired("title") + decisionsCreateCmd.MarkFlagsMutuallyExclusive("body", "body-file") + + ef := decisionsEditCmd.Flags() + ef.StringVar(&decisionsEditFlags.Type, "type", "", "change the kind of decision ("+decisionTypes+")") + ef.StringVar(&decisionsEditFlags.Title, "title", "", "change the title") + ef.StringVar(&decisionsEditFlags.Summary, "summary", "", "change the summary") + registerAnchorFlags(ef, &decisionsEditFlags.Path, &decisionsEditFlags.Lines, &decisionsEditFlags.Commit) + registerBodyFlags(ef, &decisionsEditFlags.Body, &decisionsEditFlags.BodyFile, + "also add this comment to each decision") + registerAuthorFlags(decisionsEditCmd, &decisionsEditFlags.Author, true) + decisionsEditCmd.MarkFlagsMutuallyExclusive("body", "body-file") + + registerBodyFlags(decisionsCommentCmd.Flags(), &decisionsCommentFlags.Body, &decisionsCommentFlags.BodyFile, + "the comment") + decisionsCommentCmd.Flags().IntVar(&decisionsCommentFlags.Edit, "edit", 0, + "replace the body of this comment instead of adding one") + registerAuthorFlags(decisionsCommentCmd, &decisionsCommentFlags.Author, false) + decisionsCommentCmd.MarkFlagsMutuallyExclusive("body", "body-file") + for _, name := range []string{"role", "model", "harness", "skill"} { + decisionsCommentCmd.MarkFlagsMutuallyExclusive("edit", name) + } + + registerBodyFlags(decisionsReopenCmd.Flags(), &decisionsReopenFlags.Body, &decisionsReopenFlags.BodyFile, + "add a comment saying why it was reopened") + registerAuthorFlags(decisionsReopenCmd, &decisionsReopenFlags.Author, true) + decisionsReopenCmd.MarkFlagsMutuallyExclusive("body", "body-file") + + sf := decisionsSettleCmd.Flags() + sf.StringVar(&decisionsSettleFlags.Outcome, "outcome", "", "how it was settled ("+decisionOutcomes+")") + sf.StringVar(&decisionsSettleFlags.Summary, "summary", "", "set the decision's summary of what was decided") + sf.StringVar(&decisionsSettleFlags.Resolution, "resolution", "", "why it was settled this way") + registerAuthorFlags(decisionsSettleCmd, &decisionsSettleFlags.Author, true) + _ = decisionsSettleCmd.MarkFlagRequired("outcome") + + decisionsApplyCmd.Flags().StringVar(&decisionsApplyFlags.File, "file", "", "the JSON batch file, or - for stdin") + _ = decisionsApplyCmd.MarkFlagRequired("file") + + decisionsCmd.AddCommand( + decisionsListCmd, + decisionsShowCmd, + decisionsCreateCmd, + decisionsEditCmd, + decisionsCommentCmd, + decisionsSettleCmd, + decisionsReopenCmd, + decisionsApplyCmd, + ) + rootCmd.AddCommand(decisionsCmd) +} diff --git a/cmd/aviator/decisions_format.go b/cmd/aviator/decisions_format.go new file mode 100644 index 0000000..6026753 --- /dev/null +++ b/cmd/aviator/decisions_format.go @@ -0,0 +1,190 @@ +//go:build experimental + +package main + +import ( + "fmt" + "strings" + "text/tabwriter" + + "github.com/aviator-co/aviator-cli/internal/api" + "github.com/aviator-co/aviator-cli/internal/utils/colors" + "github.com/dustin/go-humanize/english" +) + +// formatDecisionTable renders decisions one per row. Title is the last column +// so the one unpadded cell carries the long text. +func formatDecisionTable(decisions []api.Decision) string { + var b strings.Builder + w := tabwriter.NewWriter(&b, 0, 0, 2, ' ', 0) + fmt.Fprintln(w, "ID\tSTATUS\tTYPE\tCOMMENTS\tTITLE") + for i := range decisions { + d := &decisions[i] + fmt.Fprintf(w, "#%d\t%s\t%s\t%d\t%s\n", d.ID, formatDecisionStatus(d), d.Type, len(d.Comments), d.Title) + } + _ = w.Flush() + return b.String() +} + +// formatDecisionStatus renders the status with the outcome once settled. +func formatDecisionStatus(d *api.Decision) string { + if d.Status == "settled" && d.Outcome != "" { + return "settled (" + d.Outcome + ")" + } + return d.Status +} + +// formatDecisionDetail renders one decision and its comment thread. +func formatDecisionDetail(d *api.Decision) string { + var b strings.Builder + fmt.Fprintf(&b, "%s %s\n", colors.Bold(fmt.Sprintf("#%d", d.ID)), colors.Bold(d.Title)) + fmt.Fprintf(&b, " %s · %s · raised by %s · last changed as %s\n", + d.Type, formatDecisionStatus(d), d.RaisedBy.Name, formatDecisionAuthor(d.Author)) + if d.Anchor != nil { + fmt.Fprintf(&b, " at %s\n", formatDecisionAnchor(d.Anchor)) + } + if summary := strings.TrimSpace(d.Summary); summary != "" { + fmt.Fprintf(&b, "\n%s\n", indent(summary, " ")) + } + if resolution := strings.TrimSpace(d.Resolution); resolution != "" { + fmt.Fprintf(&b, "\n Resolution:\n%s\n", indent(resolution, " ")) + } + if len(d.Comments) == 0 { + return b.String() + } + fmt.Fprintf(&b, "\n%s\n", english.Plural(len(d.Comments), "comment", "")) + for _, c := range d.Comments { + fmt.Fprintf(&b, " %s %s as %s %s\n", colors.Faint(fmt.Sprintf("#%d", c.ID)), + c.User.Name, formatDecisionAuthor(c.Author), colors.Faint(c.CreatedAt)) + fmt.Fprintf(&b, "%s\n", indent(strings.TrimSpace(c.Body), " ")) + } + return b.String() +} + +// formatDecisionAuthor renders a role, with its model and harness when known. +func formatDecisionAuthor(a api.DecisionAuthor) string { + var details []string + for _, s := range []string{a.Model, a.Harness} { + if s != "" { + details = append(details, s) + } + } + if len(details) == 0 { + return a.Role + } + return fmt.Sprintf("%s (%s)", a.Role, strings.Join(details, ", ")) +} + +// formatDecisionAnchor renders an anchor as path:start-end @ short-sha. +func formatDecisionAnchor(a *api.DecisionAnchor) string { + s := a.FilePath + switch { + case a.StartLine > 0 && a.EndLine > a.StartLine: + s += fmt.Sprintf(":%d-%d", a.StartLine, a.EndLine) + case a.StartLine > 0: + s += fmt.Sprintf(":%d", a.StartLine) + } + if a.CommitSHA != "" { + sha := a.CommitSHA + if len(sha) > 7 { + sha = sha[:7] + } + s = strings.TrimSpace(s + " @ " + sha) + } + return s +} + +func indent(s, prefix string) string { + return prefix + strings.ReplaceAll(s, "\n", "\n"+prefix) +} + +type decisionAuthorJSON struct { + Role string `json:"role"` + Model string `json:"model,omitempty"` + Harness string `json:"harness,omitempty"` + Skills []string `json:"skills,omitempty"` +} + +type decisionUserJSON struct { + ID int `json:"id"` + Name string `json:"name"` +} + +type decisionAnchorJSON struct { + CommitSHA string `json:"commit_sha,omitempty"` + FilePath string `json:"file_path,omitempty"` + StartLine int `json:"start_line,omitempty"` + EndLine int `json:"end_line,omitempty"` +} + +type decisionCommentJSON struct { + ID int `json:"id"` + User decisionUserJSON `json:"user"` + Author decisionAuthorJSON `json:"author"` + Body string `json:"body"` + CreatedAt string `json:"created_at"` + ModifiedAt string `json:"modified_at"` +} + +type decisionJSON struct { + ID int `json:"id"` + Type string `json:"type"` + Title string `json:"title"` + Summary string `json:"summary"` + Status string `json:"status"` + Outcome string `json:"outcome"` + SettledAt string `json:"settled_at"` + Resolution string `json:"resolution"` + Anchor *decisionAnchorJSON `json:"anchor"` + RaisedBy decisionUserJSON `json:"raised_by"` + Author decisionAuthorJSON `json:"author"` + Comments []decisionCommentJSON `json:"comments"` +} + +type decisionListJSON struct { + Session string `json:"session"` + Decisions []decisionJSON `json:"decisions"` +} + +func newDecisionJSON(d *api.Decision) decisionJSON { + out := decisionJSON{ + ID: d.ID, + Type: d.Type, + Title: d.Title, + Summary: d.Summary, + Status: d.Status, + Outcome: d.Outcome, + SettledAt: d.SettledAt, + Resolution: d.Resolution, + RaisedBy: decisionUserJSON(d.RaisedBy), + Author: decisionAuthorJSON(d.Author), + Comments: make([]decisionCommentJSON, 0, len(d.Comments)), + } + if d.Anchor != nil { + anchor := decisionAnchorJSON(*d.Anchor) + out.Anchor = &anchor + } + for i := range d.Comments { + out.Comments = append(out.Comments, newDecisionCommentJSON(&d.Comments[i])) + } + return out +} + +func newDecisionCommentJSON(c *api.DecisionComment) decisionCommentJSON { + return decisionCommentJSON{ + ID: c.ID, + User: decisionUserJSON(c.User), + Author: decisionAuthorJSON(c.Author), + Body: c.Body, + CreatedAt: c.CreatedAt, + ModifiedAt: c.ModifiedAt, + } +} + +func newDecisionListJSON(runbookNumber int, decisions []api.Decision) decisionListJSON { + out := decisionListJSON{Session: formatRunbookID(runbookNumber), Decisions: make([]decisionJSON, 0, len(decisions))} + for i := range decisions { + out.Decisions = append(out.Decisions, newDecisionJSON(&decisions[i])) + } + return out +} diff --git a/cmd/aviator/decisions_test.go b/cmd/aviator/decisions_test.go new file mode 100644 index 0000000..ad07dc2 --- /dev/null +++ b/cmd/aviator/decisions_test.go @@ -0,0 +1,187 @@ +//go:build experimental + +package main + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/aviator-co/aviator-cli/internal/api" + "github.com/aviator-co/aviator-cli/internal/config" + "github.com/spf13/cobra" +) + +func TestParseLineRange(t *testing.T) { + for _, c := range []struct { + in string + start, end int + }{{"40", 40, 40}, {"40-52", 40, 52}, {" 3 - 4 ", 3, 4}} { + start, end, err := parseLineRange(c.in) + if err != nil || start != c.start || end != c.end { + t.Errorf("%q: %d-%d, %v", c.in, start, end, err) + } + } + for _, in := range []string{"", "x", "0", "52-40", "1-x"} { + if _, _, err := parseLineRange(in); err == nil { + t.Errorf("%q: want an error", in) + } + } +} + +func TestBuildAnchor(t *testing.T) { + if a, err := buildAnchor("", "", ""); !a.IsZero() || err != nil { + t.Errorf("no flags: %+v, %v", a, err) + } + if _, err := buildAnchor("", "4", ""); err == nil { + t.Error("--lines without --path: want an error") + } + a, err := buildAnchor("a.go", "40-52", "abc") + if err != nil || a.Value == nil || + *a.Value != (api.DecisionAnchor{FilePath: "a.go", StartLine: 40, EndLine: 52, CommitSHA: "abc"}) { + t.Errorf("anchor = %+v, %v", a, err) + } +} + +// TestDecisionCommandBodies runs write commands against a test server and +// checks each sends its whole change as one batch. It calls RunE directly: +// going through rootCmd would load the real config and point the client away +// from the test server. +func TestDecisionCommandBodies(t *testing.T) { + var got []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + data, _ := io.ReadAll(r.Body) + got = append(got, r.Method+" "+r.URL.Path+" "+string(data)) + _, _ = w.Write([]byte(`{"decisions":[{"id":7,"status":"open"},{"id":8,"status":"open"}]}`)) + })) + defer srv.Close() + host, token := config.Av.Aviator.APIHost, config.Av.Aviator.APIToken + config.Av.Aviator.APIHost, config.Av.Aviator.APIToken = srv.URL, "test-token" + t.Cleanup(func() { config.Av.Aviator.APIHost, config.Av.Aviator.APIToken = host, token }) + decisionsFlags.JSON = true + t.Cleanup(func() { decisionsFlags.JSON = false }) + + for _, c := range []struct { + cmd *cobra.Command + args []string + want string + }{ + { + decisionsSettleCmd, + []string{ + "r/4", "7", "#8", "--outcome", "fixed", "--resolution", "why", + "--role", "coding_agent", "--model", "m", "--harness", "claude-code", "--skill", "a", "--skill", "b", + }, + `POST /api/v1/verify/4/decisions/batch {"decisions":[` + + `{"id":7,"author":{"role":"coding_agent","model":"m","harness":"claude-code","skills":["a","b"]},` + + `"outcome":"fixed","resolution":"why"},` + + `{"id":8,"author":{"role":"coding_agent","model":"m","harness":"claude-code","skills":["a","b"]},` + + `"outcome":"fixed","resolution":"why"}]}`, + }, + { + decisionsReopenCmd, + []string{"r/4", "7", "--role", "person", "--body", "not fixed"}, + `POST /api/v1/verify/4/decisions/batch {"decisions":[{"id":7,"author":{"role":"person"},` + + `"outcome":null,"comments":[{"author":{"role":"person"},"body":"not fixed"}]}]}`, + }, + { + decisionsCreateCmd, + []string{ + "r/4", "--type", "risk", "--title", "T", "--body", "why", + "--path", "a.go", "--lines", "3", "--role", "review_agent", + }, + `POST /api/v1/verify/4/decisions/batch {"decisions":[{"author":{"role":"review_agent"},` + + `"type":"risk","title":"T","anchor":{"file_path":"a.go","start_line":3,"end_line":3},` + + `"comments":[{"author":{"role":"review_agent"},"body":"why"}]}]}`, + }, + } { + got = nil + c.cmd.SetContext(context.Background()) + if err := c.cmd.ParseFlags(c.args); err != nil { + t.Fatalf("%v: %v", c.args, err) + } + if err := c.cmd.RunE(c.cmd, c.cmd.Flags().Args()); err != nil { + t.Fatalf("%v: %v", c.args, err) + } + if len(got) != 1 || got[0] != c.want { + t.Errorf("%v: requests =\n%s\nwant\n%s", c.args, strings.Join(got, "\n"), c.want) + } + } +} + +func TestReadDecisionBatch(t *testing.T) { + write := func(content string) string { + path := filepath.Join(t.TempDir(), "batch.json") + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + return path + } + req, err := readDecisionBatch(write(`{"decisions":[` + + `{"author":{"role":"coding_agent","model":"m"},"type":"risk","title":"T",` + + `"comments":[{"author":{"role":"coding_agent"},"body":"b","model_metadata":{"subagent":"r"}}]},` + + `{"id":8,"author":{"role":"person"},"outcome":"fixed","resolution":"why"},` + + `{"id":9,"author":{"role":"person"},"outcome":null,"anchor":null}]}`)) + if err != nil || len(req.Decisions) != 3 { + t.Fatalf("req = %+v, err = %v", req, err) + } + if d := req.Decisions[0]; d.Author.Model != "m" || d.Outcome.Set || d.Comments[0].ModelMetadata["subagent"] != "r" { + t.Errorf("create = %+v", d) + } + if d := req.Decisions[1]; *d.Outcome.Value != "fixed" || d.Resolution != "why" { + t.Errorf("settle = %+v", d) + } + if d := req.Decisions[2]; !d.Outcome.Set || d.Outcome.Value != nil || !d.Anchor.Set || d.Anchor.Value != nil { + t.Errorf("reopen = %+v", d) + } + for name, content := range map[string]string{ + "via": `{"via":"aviator-cli","decisions":[{"id":7,"author":{"role":"person"}}]}`, + "batch model_metadata": `{"model_metadata":{"model":"m"},"decisions":[{"id":7,"author":{"role":"person"}}]}`, + "unknown key": `{"decisions":[{"id":7,"author":{"role":"person"},"coments":[]}]}`, + "old nested format": `{"decisions":[{"decision":{"id":7}}]}`, + "author name": `{"decisions":[{"id":7,"author":{"role":"person","name":"Sam"}}]}`, + "bare array": `[{"id":7}]`, + "empty": `{"decisions":[]}`, + } { + if _, err := readDecisionBatch(write(content)); err == nil { + t.Errorf("%s: want an error", name) + } + } +} + +func TestFormatDecisionAnchor(t *testing.T) { + for want, a := range map[string]api.DecisionAnchor{ + "a.go:40-52 @ abcdef1": {FilePath: "a.go", StartLine: 40, EndLine: 52, CommitSHA: "abcdef1234"}, + "a.go:40": {FilePath: "a.go", StartLine: 40, EndLine: 40}, + "@ abc": {CommitSHA: "abc"}, + } { + if got := formatDecisionAnchor(&a); got != want { + t.Errorf("got %q, want %q", got, want) + } + } +} + +func TestFormatDecisionDetail(t *testing.T) { + got := formatDecisionDetail(&api.Decision{ + ID: 7, Type: "risk", Title: "T", Status: "settled", Outcome: "accepted_risk", Resolution: "Rare enough", + RaisedBy: api.DecisionUser{Name: "Theo"}, + Author: api.DecisionAuthor{Role: "coding_agent", Model: "m", Harness: "claude-code"}, + Comments: []api.DecisionComment{{ + ID: 1, User: api.DecisionUser{Name: "Theo"}, Author: api.DecisionAuthor{Role: "person"}, Body: "hi", + }}, + }) + for _, want := range []string{ + "raised by Theo · last changed as coding_agent (m, claude-code)", + "Resolution:\n Rare enough", + "Theo as person", + } { + if !strings.Contains(got, want) { + t.Errorf("detail lacks %q:\n%s", want, got) + } + } +} diff --git a/cmd/aviator/experimental_test.go b/cmd/aviator/experimental_test.go new file mode 100644 index 0000000..e689360 --- /dev/null +++ b/cmd/aviator/experimental_test.go @@ -0,0 +1,29 @@ +//go:build experimental + +package main + +import ( + "bytes" + "strings" + "testing" +) + +// TestExperimentalCommandsHidden keeps the experimental commands out of help +// and shell completion, which Homebrew builds from `aviator completion`. +func TestExperimentalCommandsHidden(t *testing.T) { + for _, args := range [][]string{{"--help"}, {"help"}, {"__complete", ""}} { + var out bytes.Buffer + rootCmd.SetOut(&out) + rootCmd.SetArgs(args) + if err := rootCmd.Execute(); err != nil { + t.Fatalf("%v: %v", args, err) + } + for _, name := range []string{"decision"} { + if strings.Contains(strings.ToLower(out.String()), name) { + t.Errorf("%v lists %q:\n%s", args, name, out.String()) + } + } + } + rootCmd.SetOut(nil) + rootCmd.SetArgs(nil) +} diff --git a/internal/api/decisions.go b/internal/api/decisions.go new file mode 100644 index 0000000..e583709 --- /dev/null +++ b/internal/api/decisions.go @@ -0,0 +1,179 @@ +//go:build experimental + +package api + +import ( + "context" + "encoding/json" + "fmt" +) + +// DecisionAuthor is the role, and the agent details when known, behind a +// decision write or a comment. Role is person, coding_agent, review_agent, or +// external_bot. +type DecisionAuthor struct { + Role string `json:"role"` + Model string `json:"model,omitempty"` + Harness string `json:"harness,omitempty"` + Skills []string `json:"skills,omitempty"` +} + +// DecisionUser is the Aviator user behind a decision or comment. +type DecisionUser struct { + ID int `json:"id"` + Name string `json:"name"` +} + +// DecisionAnchor pins a decision to lines of a file at a commit. +type DecisionAnchor struct { + CommitSHA string `json:"commit_sha,omitempty"` + FilePath string `json:"file_path,omitempty"` + StartLine int `json:"start_line,omitempty"` + EndLine int `json:"end_line,omitempty"` +} + +// DecisionComment is one entry in a decision's thread. User is the account +// that wrote it; Author is the role it was written as. +type DecisionComment struct { + ID int `json:"id"` + User DecisionUser `json:"user"` + Author DecisionAuthor `json:"author"` + Body string `json:"body"` + CreatedAt string `json:"created_at"` + ModifiedAt string `json:"modified_at"` +} + +// Decision is a question, possible bug, risk, or choice raised about a +// runbook's pull request. Status is open or settled; Outcome, SettledAt, and +// Resolution are set once it is settled. RaisedBy is the user who created it +// and never changes; Author is the author of the latest change. Comments are +// in creation order. +type Decision struct { + ID int `json:"id"` + Type string `json:"type"` + Title string `json:"title"` + Summary string `json:"summary"` + Status string `json:"status"` + Outcome string `json:"outcome"` + SettledAt string `json:"settled_at"` + Resolution string `json:"resolution"` + Anchor *DecisionAnchor `json:"anchor"` + RaisedBy DecisionUser `json:"raised_by"` + Author DecisionAuthor `json:"author"` + Comments []DecisionComment `json:"comments"` +} + +// DecisionBatchRequest is the body for POST +// /api/v1/verify//decisions/batch. The items are applied all-or-nothing. +type DecisionBatchRequest struct { + Decisions []DecisionBatchItem `json:"decisions"` +} + +// DecisionBatchItem creates a decision when ID is zero (Type and Title are +// required), and otherwise changes only the keys it sends. Author is credited +// with the creation or change. An Outcome settles the decision and a null one +// reopens it; a null Anchor clears it. +type DecisionBatchItem struct { + ID int `json:"id,omitempty"` + Author DecisionAuthor `json:"author,omitzero"` + Type string `json:"type,omitempty"` + Title string `json:"title,omitempty"` + Summary string `json:"summary,omitempty"` + Anchor Nullable[DecisionAnchor] `json:"anchor,omitzero"` + Outcome Nullable[string] `json:"outcome,omitzero"` + Resolution string `json:"resolution,omitempty"` + Comments []DecisionCommentInput `json:"comments,omitempty"` +} + +// DecisionCommentInput is one comment to add. ModelMetadata is free-form agent +// info the server stores but never returns. +type DecisionCommentInput struct { + Author DecisionAuthor `json:"author,omitzero"` + Body string `json:"body"` + ModelMetadata map[string]any `json:"model_metadata,omitempty"` +} + +// Nullable is a key that can be left out, set to a value, or sent as null. +// The zero value is left out (with the omitzero tag). +type Nullable[T any] struct { + Set bool + Value *T +} + +// Null returns a Nullable that is sent as null. +func Null[T any]() Nullable[T] { return Nullable[T]{Set: true} } + +// ValueOf returns a Nullable that is sent as v. +func ValueOf[T any](v T) Nullable[T] { return Nullable[T]{Set: true, Value: &v} } + +// IsZero reports whether the key is left out. +func (n Nullable[T]) IsZero() bool { return !n.Set } + +// MarshalJSON writes the value, or null. +func (n Nullable[T]) MarshalJSON() ([]byte, error) { return json.Marshal(n.Value) } + +// UnmarshalJSON marks the key present, keeping null apart from a left-out key. +func (n *Nullable[T]) UnmarshalJSON(data []byte) error { + *n = Nullable[T]{Set: true} + return json.Unmarshal(data, &n.Value) +} + +// DecisionsResponse is the response from GET /api/v1/verify//decisions, +// open decisions first, and from the batch POST, in request order. +type DecisionsResponse struct { + Decisions []Decision `json:"decisions"` +} + +// ListDecisions fetches a runbook's decisions. +func (c *Client) ListDecisions(ctx context.Context, runbookNumber int) (*DecisionsResponse, error) { + var out DecisionsResponse + if err := c.getJSON(ctx, decisionsPath(runbookNumber), nil, &out); err != nil { + return nil, err + } + return &out, nil +} + +// GetDecision fetches one decision with its comment thread. +func (c *Client) GetDecision(ctx context.Context, runbookNumber, decisionID int) (*Decision, error) { + var out Decision + if err := c.getJSON(ctx, decisionPath(runbookNumber, decisionID), nil, &out); err != nil { + return nil, err + } + return &out, nil +} + +// ApplyDecisions creates, changes, settles, and comments on decisions in one +// all-or-nothing batch, returning each item's decision in request order. +func (c *Client) ApplyDecisions( + ctx context.Context, runbookNumber int, req DecisionBatchRequest, +) (*DecisionsResponse, error) { + var out DecisionsResponse + if err := c.postJSON(ctx, decisionsPath(runbookNumber)+"/batch", req, &out); err != nil { + return nil, err + } + return &out, nil +} + +func decisionsPath(runbookNumber int) string { + return fmt.Sprintf("/api/v1/verify/%d/decisions", runbookNumber) +} + +func decisionPath(runbookNumber, decisionID int) string { + return fmt.Sprintf("%s/%d", decisionsPath(runbookNumber), decisionID) +} + +// EditDecisionComment replaces a comment's body. Only the user who wrote the +// comment can edit it. +func (c *Client) EditDecisionComment( + ctx context.Context, runbookNumber, decisionID, commentID int, body string, +) (*DecisionComment, error) { + req := struct { + Body string `json:"body"` + }{Body: body} + var out DecisionComment + path := fmt.Sprintf("%s/comments/%d", decisionPath(runbookNumber, decisionID), commentID) + if err := c.patchJSON(ctx, path, req, &out); err != nil { + return nil, err + } + return &out, nil +} diff --git a/internal/api/decisions_test.go b/internal/api/decisions_test.go new file mode 100644 index 0000000..e0c2c47 --- /dev/null +++ b/internal/api/decisions_test.go @@ -0,0 +1,133 @@ +//go:build experimental + +package api + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" +) + +const decisionJSON = `{"id":7,"type":"risk","title":"Repos share one rate limit","summary":null,` + + `"status":"settled","outcome":"accepted_risk","settled_at":"2026-10-21T00:00:00Z",` + + `"resolution":"Rare enough to ship",` + + `"anchor":{"commit_sha":"abc","file_path":"a.go","start_line":40,"end_line":52},` + + `"repo":{"org":"acme","name":"web"},"raised_by":{"id":7,"name":"Theo"},` + + `"author":{"role":"coding_agent","model":"claude-fable-5-1","harness":"claude-code","skills":["go"]},` + + `"comments":[{"id":1,"user":{"id":7,"name":"Theo"},` + + `"author":{"role":"person","model":null,"harness":null,"skills":null},"body":"hi",` + + `"created_at":"2026-10-21T00:00:00Z","modified_at":"2026-10-21T00:00:00Z"}],` + + `"created_at":"2026-10-21T00:00:00Z","modified_at":"2026-10-21T00:00:00Z"}` + +func TestListDecisions(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || r.URL.Path != "/api/v1/verify/123/decisions" { + t.Errorf("%s %s", r.Method, r.URL.Path) + } + _, _ = w.Write([]byte(`{"decisions":[` + decisionJSON + `]}`)) + })) + defer srv.Close() + + resp, err := newTestClient(srv).ListDecisions(context.Background(), 123) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(resp.Decisions) != 1 { + t.Fatalf("decisions = %+v", resp.Decisions) + } + d := resp.Decisions[0] + if d.ID != 7 || d.Outcome != "accepted_risk" || d.Resolution != "Rare enough to ship" || + d.Anchor.EndLine != 52 || d.RaisedBy.Name != "Theo" || d.Author.Harness != "claude-code" || + len(d.Comments) != 1 || d.Comments[0].User.Name != "Theo" || d.Comments[0].Author.Role != "person" { + t.Errorf("decision = %+v", d) + } +} + +// recordRequests serves reply to every request and records each one as +// "METHOD path body". +func recordRequests(t *testing.T, reply string) (*httptest.Server, *[]string) { + t.Helper() + var got []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + data, _ := io.ReadAll(r.Body) + got = append(got, r.Method+" "+r.URL.Path+" "+string(data)) + _, _ = w.Write([]byte(reply)) + })) + t.Cleanup(srv.Close) + return srv, &got +} + +func TestGetDecision(t *testing.T) { + srv, got := recordRequests(t, decisionJSON) + d, err := newTestClient(srv).GetDecision(context.Background(), 123, 7) + if err != nil || d.ID != 7 || len(d.Comments) != 1 { + t.Fatalf("decision = %+v, err = %v", d, err) + } + if want := "GET /api/v1/verify/123/decisions/7 "; (*got)[0] != want { + t.Errorf("request = %q, want %q", (*got)[0], want) + } +} + +// TestApplyDecisionsBody checks the batch body: left-out keys stay out, and a +// null outcome or anchor is sent as null. +func TestApplyDecisionsBody(t *testing.T) { + srv, got := recordRequests(t, `{"decisions":[`+decisionJSON+`]}`) + agent := DecisionAuthor{Role: "coding_agent", Model: "m", Harness: "claude-code", Skills: []string{"go"}} + resp, err := newTestClient(srv).ApplyDecisions(context.Background(), 123, DecisionBatchRequest{ + Decisions: []DecisionBatchItem{ + { + Author: agent, Type: "choice", Title: "T", Summary: "S", + Anchor: ValueOf(DecisionAnchor{FilePath: "a.go", StartLine: 4, EndLine: 4}), + Outcome: ValueOf("decided"), + Comments: []DecisionCommentInput{{Author: agent, Body: "b", ModelMetadata: map[string]any{"subagent": "r"}}}, + }, + {ID: 7, Author: DecisionAuthor{Role: "person"}, Outcome: Null[string](), Anchor: Null[DecisionAnchor]()}, + {ID: 8, Author: DecisionAuthor{Role: "person"}, Outcome: ValueOf("fixed"), Resolution: "why"}, + }, + }) + if err != nil || len(resp.Decisions) != 1 { + t.Fatalf("resp = %+v, err = %v", resp, err) + } + want := `POST /api/v1/verify/123/decisions/batch {"decisions":[` + + `{"author":{"role":"coding_agent","model":"m","harness":"claude-code","skills":["go"]},` + + `"type":"choice","title":"T","summary":"S","anchor":{"file_path":"a.go","start_line":4,"end_line":4},` + + `"outcome":"decided","comments":[{"author":{"role":"coding_agent","model":"m","harness":"claude-code",` + + `"skills":["go"]},"body":"b","model_metadata":{"subagent":"r"}}]},` + + `{"id":7,"author":{"role":"person"},"anchor":null,"outcome":null},` + + `{"id":8,"author":{"role":"person"},"outcome":"fixed","resolution":"why"}]}` + if (*got)[0] != want { + t.Errorf("request =\n%s\nwant\n%s", (*got)[0], want) + } +} + +func TestNullableUnmarshal(t *testing.T) { + for in, want := range map[string]Nullable[string]{ + `{}`: {}, + `{"outcome":null}`: Null[string](), + `{"outcome":"x"}`: ValueOf("x"), + } { + var item DecisionBatchItem + if err := json.Unmarshal([]byte(in), &item); err != nil { + t.Fatalf("%s: %v", in, err) + } + got := item.Outcome + if got.Set != want.Set || (got.Value == nil) != (want.Value == nil) || + (got.Value != nil && *got.Value != *want.Value) { + t.Errorf("%s: outcome = %+v, want %+v", in, got, want) + } + } +} + +func TestEditDecisionCommentBody(t *testing.T) { + srv, got := recordRequests(t, `{"id":23,"user":{"id":7,"name":"Theo"},"author":{"role":"person"},"body":"new"}`) + c, err := newTestClient(srv).EditDecisionComment(context.Background(), 123, 7, 23, "new") + if err != nil || c.ID != 23 || c.Body != "new" || c.User.Name != "Theo" { + t.Fatalf("comment = %+v, err = %v", c, err) + } + if want := `PATCH /api/v1/verify/123/decisions/7/comments/23 {"body":"new"}`; (*got)[0] != want { + t.Errorf("request =\n%s\nwant\n%s", (*got)[0], want) + } +} From f0e1aa7505a779673bc68d19c687bba5df14fc82 Mon Sep 17 00:00:00 2001 From: tr-heuser Date: Wed, 28 Oct 2026 14:37:01 -0700 Subject: [PATCH 3/4] hide decisions instead of tagging it, and fix anchor edits Drop the experimental build tag: decisions is Hidden, which keeps it out of help and completion, so CI, just test, and lint run once again and no separate build is needed. edit now lays --path, --lines, and --commit over each decision's current anchor, since the server replaces an anchor as a whole and a bare --commit used to wipe the path and lines. show and --json leave out a null raised_by, author, or comment user instead of printing it blank. --- .github/workflows/go.yml | 6 -- .golangci.yaml | 5 -- CLAUDE.md | 29 +------ cmd/aviator/decisions.go | 86 +++++++++++++++---- cmd/aviator/decisions_format.go | 69 +++++++++++---- cmd/aviator/decisions_test.go | 70 +++++++++++---- .../{experimental_test.go => hidden_test.go} | 6 +- internal/api/decisions.go | 24 +++--- internal/api/decisions_test.go | 13 ++- internal/config/experimental.go | 7 -- internal/config/experimental_off.go | 7 -- justfile | 7 +- 12 files changed, 202 insertions(+), 127 deletions(-) rename cmd/aviator/{experimental_test.go => hidden_test.go} (78%) delete mode 100644 internal/config/experimental.go delete mode 100644 internal/config/experimental_off.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 9cfef82..eb26d5d 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -30,12 +30,6 @@ jobs: run: go test -v --vet=all ./... - name: CLI smoke test run: go run ./cmd/aviator --help - - name: Build (experimental) - run: go build -v -tags experimental ./... - - name: Test (experimental) - run: go test -v --vet=all -tags experimental ./... - - name: CLI smoke test (experimental) - run: go run -tags experimental ./cmd/aviator --help golangci-lint: runs-on: ubuntu-latest diff --git a/.golangci.yaml b/.golangci.yaml index 5cca199..c0836b3 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,9 +1,4 @@ version: "2" -run: - # Lint the files behind the experimental build tag too; the !experimental - # stubs are trivial and covered by the untagged build. - build-tags: - - experimental linters: enable: - asasalint diff --git a/CLAUDE.md b/CLAUDE.md index c68df66..357b5a9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,7 +29,7 @@ cmd/aviator/ # CLI entry point + commands (one file per command) evidence.go # `aviator evidence` -> signed URL / download of one evidence file invariants.go # `aviator invariants ` invariants_format.go # text rendering for the invariants commands - decisions.go # `aviator decisions ` (experimental build only) + decisions.go # `aviator decisions ` (hidden) decisions_format.go # text rendering for the decisions commands version.go # `aviator version` helpers.go # parseRepo(s), readSpecFile, collectCriteria, collectBody @@ -96,28 +96,6 @@ Equivalent raw commands: `go build ./...`, `go test --vet=all ./...`, no `formatters` block: golangci-lint bundles its own older copies of both, and two copies grading the same files is how CI deadlocks. -## Experimental features - -New features that are only for internal dogfooding sit behind the -`experimental` Go build tag, so they are left out of stable binaries entirely. -Nobody can switch them on with a flag or env var. - -- A new command goes in its own file that starts with `//go:build experimental` - and registers itself: `func init() { rootCmd.AddCommand(fooCmd) }`. Don't - list it in `main.go`. -- Mark a new top-level command `Hidden: true`, so even experimental builds - leave it out of `--help`, `aviator help`, and shell completion. It still runs - when called by name. -- A new flag or branch inside an existing command is gated on - `config.Experimental` (a const, so the stable build drops the dead code). -- No release (stable or nightly) is built with the tag; both are public. The - team builds its own binary with `just build-experimental` or - `go run -tags experimental ./cmd/aviator`. -- CI builds, tests, and smoke-tests both variants; golangci-lint runs with the - tag on. -- To ship a feature to everyone, delete the build tag or the `config.Experimental` - check. - ## CI and releases - `.github/workflows/go.yml` — build, test, smoke test, golangci-lint on PRs. @@ -182,8 +160,9 @@ The CLI targets endpoints in the `mergeit` backend: The CLI doesn't follow it with the bearer token: `EvidenceURL` stops at the redirect and the download is a plain unauthenticated GET. -- `/api/v1/verify//decisions` — experimental build only. Every body - rejects unknown keys. +- `/api/v1/verify//decisions` — for `aviator decisions`, which is + `Hidden: true` while it changes (out of help and completion, but it runs + when called by name). Every body rejects unknown keys. - `GET` — `{decisions: [...]}`, open first. `GET /` — one decision. Reads take any API token. - `POST /batch` — `{decisions: [{id?, author, type?, title?, summary?, diff --git a/cmd/aviator/decisions.go b/cmd/aviator/decisions.go index cc661ee..a71de09 100644 --- a/cmd/aviator/decisions.go +++ b/cmd/aviator/decisions.go @@ -1,5 +1,3 @@ -//go:build experimental - package main import ( @@ -48,7 +46,8 @@ var decisionsCmd = &cobra.Command{ Use: "decisions", Aliases: []string{"decision"}, Short: "Read and record the decisions on a session's pull request", - // Experimental: left out of help and completion. + // Still changing, so left out of help and completion; it runs when called + // by name. Hidden: true, Long: "A decision is a question, possible bug, risk, or choice raised about a\n" + "session's pull request. Each one has a comment thread and is either open\n" + @@ -151,7 +150,7 @@ var decisionsCreateCmd = &cobra.Command{ if body == "" { return errors.New("--body (or --body-file) is required") } - anchor, err := buildAnchor(f.Path, f.Lines, f.Commit) + anchor, err := buildAnchor(nil, f.Path, f.Lines, f.Commit) if err != nil { return err } @@ -183,8 +182,10 @@ var decisionsEditCmd = &cobra.Command{ Use: "edit ...", Short: "Change open decisions; every listed one gets the same changes", Long: "Change the type, title, summary, or anchor of open decisions as\n" + - "--role. --body also adds a comment to each one's thread. Only the user\n" + - "who raised a decision can edit it, and settled decisions can't be edited.", + "--role. --path, --lines, and --commit change only those parts of each\n" + + "decision's current anchor. --body also adds a comment to each one's\n" + + "thread. Only the user who raised a decision can edit it, and settled\n" + + "decisions can't be edited.", Args: cobra.MinimumNArgs(2), RunE: func(cmd *cobra.Command, args []string) error { f := &decisionsEditFlags @@ -192,13 +193,16 @@ var decisionsEditCmd = &cobra.Command{ if err != nil { return err } - anchor, err := buildAnchor(f.Path, f.Lines, f.Commit) - if err != nil { - return err - } - if f.Type == "" && f.Title == "" && f.Summary == "" && anchor.IsZero() && body == "" { + anchorSet := f.Path != "" || f.Lines != "" || f.Commit != "" + if f.Type == "" && f.Title == "" && f.Summary == "" && !anchorSet && body == "" { return errors.New("nothing to change: pass --type, --title, --summary, an anchor, or --body") } + anchors := map[int]api.Nullable[api.DecisionAnchor]{} + if anchorSet { + if anchors, err = mergedAnchors(cmd, args, f.Path, f.Lines, f.Commit); err != nil { + return err + } + } author := f.Author.author() return recordForEach(cmd, args, func(id int) api.DecisionBatchItem { item := api.DecisionBatchItem{ @@ -207,7 +211,7 @@ var decisionsEditCmd = &cobra.Command{ Type: f.Type, Title: f.Title, Summary: f.Summary, - Anchor: anchor, + Anchor: anchors[id], } if body != "" { item.Comments = []api.DecisionCommentInput{{Author: author, Body: body}} @@ -432,16 +436,59 @@ func editDecisionComment(cmd *cobra.Command, args []string, commentID int, body return nil } -// buildAnchor turns the anchor flags into an anchor, left out when none were -// passed. -func buildAnchor(path, lines, commit string) (api.Nullable[api.DecisionAnchor], error) { +// mergedAnchors fetches the session in args[0] once and returns, for each +// decision ID in args[1:], its current anchor with the anchor flags laid over +// it. An ID the session doesn't have gets the flags alone; the batch then +// reports it as not found. +func mergedAnchors( + cmd *cobra.Command, args []string, path, lines, commit string, +) (map[int]api.Nullable[api.DecisionAnchor], error) { + runbookNumber, err := parseRunbookID(args[0]) + if err != nil { + return nil, err + } + client, err := api.NewClient() + if err != nil { + return nil, err + } + resp, err := client.ListDecisions(cmd.Context(), runbookNumber) + if err != nil { + return nil, errors.Wrap(err, "failed to fetch the current anchors") + } + current := make(map[int]*api.DecisionAnchor, len(resp.Decisions)) + for _, d := range resp.Decisions { + current[d.ID] = d.Anchor + } + anchors := make(map[int]api.Nullable[api.DecisionAnchor], len(args)-1) + for _, arg := range args[1:] { + id, err := parseDecisionID(arg) + if err != nil { + return nil, err + } + if anchors[id], err = buildAnchor(current[id], path, lines, commit); err != nil { + return nil, errors.Wrapf(err, "#%d", id) + } + } + return anchors, nil +} + +// buildAnchor lays the anchor flags over base (nil when there is none), so a +// flag left out keeps base's value: the server replaces an anchor as a whole. +// The anchor is left out when no flags were passed. +func buildAnchor(base *api.DecisionAnchor, path, lines, commit string) (api.Nullable[api.DecisionAnchor], error) { if path == "" && lines == "" && commit == "" { return api.Nullable[api.DecisionAnchor]{}, nil } - if lines != "" && path == "" { - return api.Nullable[api.DecisionAnchor]{}, errors.New("--lines needs --path") + var anchor api.DecisionAnchor + if base != nil { + anchor = *base + } + if path != "" { + anchor.FilePath = path + } + if commit != "" { + anchor.CommitSHA = commit } - anchor := api.DecisionAnchor{FilePath: path, CommitSHA: commit} if lines != "" { start, end, err := parseLineRange(lines) if err != nil { @@ -449,6 +496,9 @@ func buildAnchor(path, lines, commit string) (api.Nullable[api.DecisionAnchor], } anchor.StartLine, anchor.EndLine = start, end } + if anchor.StartLine > 0 && anchor.FilePath == "" { + return api.Nullable[api.DecisionAnchor]{}, errors.New("--lines needs --path") + } return api.ValueOf(anchor), nil } diff --git a/cmd/aviator/decisions_format.go b/cmd/aviator/decisions_format.go index 6026753..e9d0df7 100644 --- a/cmd/aviator/decisions_format.go +++ b/cmd/aviator/decisions_format.go @@ -1,5 +1,3 @@ -//go:build experimental - package main import ( @@ -38,8 +36,14 @@ func formatDecisionStatus(d *api.Decision) string { func formatDecisionDetail(d *api.Decision) string { var b strings.Builder fmt.Fprintf(&b, "%s %s\n", colors.Bold(fmt.Sprintf("#%d", d.ID)), colors.Bold(d.Title)) - fmt.Fprintf(&b, " %s · %s · raised by %s · last changed as %s\n", - d.Type, formatDecisionStatus(d), d.RaisedBy.Name, formatDecisionAuthor(d.Author)) + parts := []string{d.Type, formatDecisionStatus(d)} + if d.RaisedBy != nil { + parts = append(parts, "raised by "+d.RaisedBy.Name) + } + if d.Author != nil { + parts = append(parts, "last changed as "+formatDecisionAuthor(*d.Author)) + } + fmt.Fprintf(&b, " %s\n", strings.Join(parts, " · ")) if d.Anchor != nil { fmt.Fprintf(&b, " at %s\n", formatDecisionAnchor(d.Anchor)) } @@ -54,13 +58,26 @@ func formatDecisionDetail(d *api.Decision) string { } fmt.Fprintf(&b, "\n%s\n", english.Plural(len(d.Comments), "comment", "")) for _, c := range d.Comments { - fmt.Fprintf(&b, " %s %s as %s %s\n", colors.Faint(fmt.Sprintf("#%d", c.ID)), - c.User.Name, formatDecisionAuthor(c.Author), colors.Faint(c.CreatedAt)) + fmt.Fprintf(&b, " %s %s %s\n", colors.Faint(fmt.Sprintf("#%d", c.ID)), + formatCommentBy(&c), colors.Faint(c.CreatedAt)) fmt.Fprintf(&b, "%s\n", indent(strings.TrimSpace(c.Body), " ")) } return b.String() } +// formatCommentBy names who wrote a comment and as what role, leaving out +// whichever the server sent as null. +func formatCommentBy(c *api.DecisionComment) string { + var parts []string + if c.User != nil { + parts = append(parts, c.User.Name) + } + if c.Author != nil { + parts = append(parts, "as "+formatDecisionAuthor(*c.Author)) + } + return strings.Join(parts, " ") +} + // formatDecisionAuthor renders a role, with its model and harness when known. func formatDecisionAuthor(a api.DecisionAuthor) string { var details []string @@ -118,12 +135,12 @@ type decisionAnchorJSON struct { } type decisionCommentJSON struct { - ID int `json:"id"` - User decisionUserJSON `json:"user"` - Author decisionAuthorJSON `json:"author"` - Body string `json:"body"` - CreatedAt string `json:"created_at"` - ModifiedAt string `json:"modified_at"` + ID int `json:"id"` + User *decisionUserJSON `json:"user"` + Author *decisionAuthorJSON `json:"author"` + Body string `json:"body"` + CreatedAt string `json:"created_at"` + ModifiedAt string `json:"modified_at"` } type decisionJSON struct { @@ -136,8 +153,8 @@ type decisionJSON struct { SettledAt string `json:"settled_at"` Resolution string `json:"resolution"` Anchor *decisionAnchorJSON `json:"anchor"` - RaisedBy decisionUserJSON `json:"raised_by"` - Author decisionAuthorJSON `json:"author"` + RaisedBy *decisionUserJSON `json:"raised_by"` + Author *decisionAuthorJSON `json:"author"` Comments []decisionCommentJSON `json:"comments"` } @@ -146,6 +163,22 @@ type decisionListJSON struct { Decisions []decisionJSON `json:"decisions"` } +func newDecisionUserJSON(u *api.DecisionUser) *decisionUserJSON { + if u == nil { + return nil + } + out := decisionUserJSON(*u) + return &out +} + +func newDecisionAuthorJSON(a *api.DecisionAuthor) *decisionAuthorJSON { + if a == nil { + return nil + } + out := decisionAuthorJSON(*a) + return &out +} + func newDecisionJSON(d *api.Decision) decisionJSON { out := decisionJSON{ ID: d.ID, @@ -156,8 +189,8 @@ func newDecisionJSON(d *api.Decision) decisionJSON { Outcome: d.Outcome, SettledAt: d.SettledAt, Resolution: d.Resolution, - RaisedBy: decisionUserJSON(d.RaisedBy), - Author: decisionAuthorJSON(d.Author), + RaisedBy: newDecisionUserJSON(d.RaisedBy), + Author: newDecisionAuthorJSON(d.Author), Comments: make([]decisionCommentJSON, 0, len(d.Comments)), } if d.Anchor != nil { @@ -173,8 +206,8 @@ func newDecisionJSON(d *api.Decision) decisionJSON { func newDecisionCommentJSON(c *api.DecisionComment) decisionCommentJSON { return decisionCommentJSON{ ID: c.ID, - User: decisionUserJSON(c.User), - Author: decisionAuthorJSON(c.Author), + User: newDecisionUserJSON(c.User), + Author: newDecisionAuthorJSON(c.Author), Body: c.Body, CreatedAt: c.CreatedAt, ModifiedAt: c.ModifiedAt, diff --git a/cmd/aviator/decisions_test.go b/cmd/aviator/decisions_test.go index ad07dc2..2f61cc4 100644 --- a/cmd/aviator/decisions_test.go +++ b/cmd/aviator/decisions_test.go @@ -1,5 +1,3 @@ -//go:build experimental - package main import ( @@ -35,16 +33,31 @@ func TestParseLineRange(t *testing.T) { } func TestBuildAnchor(t *testing.T) { - if a, err := buildAnchor("", "", ""); !a.IsZero() || err != nil { + base := &api.DecisionAnchor{FilePath: "a.go", StartLine: 40, EndLine: 52, CommitSHA: "abc"} + if a, err := buildAnchor(base, "", "", ""); !a.IsZero() || err != nil { t.Errorf("no flags: %+v, %v", a, err) } - if _, err := buildAnchor("", "4", ""); err == nil { + if _, err := buildAnchor(nil, "", "4", ""); err == nil { t.Error("--lines without --path: want an error") } - a, err := buildAnchor("a.go", "40-52", "abc") - if err != nil || a.Value == nil || - *a.Value != (api.DecisionAnchor{FilePath: "a.go", StartLine: 40, EndLine: 52, CommitSHA: "abc"}) { - t.Errorf("anchor = %+v, %v", a, err) + for _, c := range []struct { + base *api.DecisionAnchor + path, lines, commit string + want api.DecisionAnchor + }{ + {nil, "a.go", "40-52", "abc", *base}, + // A flag changes only its part; the rest comes from the current anchor. + {base, "", "", "def", api.DecisionAnchor{FilePath: "a.go", StartLine: 40, EndLine: 52, CommitSHA: "def"}}, + {base, "", "7", "", api.DecisionAnchor{FilePath: "a.go", StartLine: 7, EndLine: 7, CommitSHA: "abc"}}, + {base, "b.go", "", "", api.DecisionAnchor{FilePath: "b.go", StartLine: 40, EndLine: 52, CommitSHA: "abc"}}, + } { + a, err := buildAnchor(c.base, c.path, c.lines, c.commit) + if err != nil || a.Value == nil || *a.Value != c.want { + t.Errorf("%+v: anchor = %+v, %v", c, a.Value, err) + } + } + if *base != (api.DecisionAnchor{FilePath: "a.go", StartLine: 40, EndLine: 52, CommitSHA: "abc"}) { + t.Errorf("base was changed: %+v", *base) } } @@ -57,7 +70,9 @@ func TestDecisionCommandBodies(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { data, _ := io.ReadAll(r.Body) got = append(got, r.Method+" "+r.URL.Path+" "+string(data)) - _, _ = w.Write([]byte(`{"decisions":[{"id":7,"status":"open"},{"id":8,"status":"open"}]}`)) + _, _ = w.Write([]byte(`{"decisions":[{"id":7,"status":"open",` + + `"anchor":{"commit_sha":"abc","file_path":"a.go","start_line":40,"end_line":52}},` + + `{"id":8,"status":"open","anchor":null}]}`)) })) defer srv.Close() host, token := config.Av.Aviator.APIHost, config.Av.Aviator.APIToken @@ -83,6 +98,16 @@ func TestDecisionCommandBodies(t *testing.T) { `{"id":8,"author":{"role":"coding_agent","model":"m","harness":"claude-code","skills":["a","b"]},` + `"outcome":"fixed","resolution":"why"}]}`, }, + { + // The anchor flags are merged into each decision's current anchor, + // read in one GET, and the change is still one batch. + decisionsEditCmd, + []string{"r/4", "7", "8", "--commit", "def", "--role", "person"}, + "GET /api/v1/verify/4/decisions \n" + + `POST /api/v1/verify/4/decisions/batch {"decisions":[` + + `{"id":7,"author":{"role":"person"},"anchor":{"commit_sha":"def","file_path":"a.go","start_line":40,"end_line":52}},` + + `{"id":8,"author":{"role":"person"},"anchor":{"commit_sha":"def"}}]}`, + }, { decisionsReopenCmd, []string{"r/4", "7", "--role", "person", "--body", "not fixed"}, @@ -108,7 +133,7 @@ func TestDecisionCommandBodies(t *testing.T) { if err := c.cmd.RunE(c.cmd, c.cmd.Flags().Args()); err != nil { t.Fatalf("%v: %v", c.args, err) } - if len(got) != 1 || got[0] != c.want { + if strings.Join(got, "\n") != c.want { t.Errorf("%v: requests =\n%s\nwant\n%s", c.args, strings.Join(got, "\n"), c.want) } } @@ -169,14 +194,15 @@ func TestFormatDecisionAnchor(t *testing.T) { func TestFormatDecisionDetail(t *testing.T) { got := formatDecisionDetail(&api.Decision{ ID: 7, Type: "risk", Title: "T", Status: "settled", Outcome: "accepted_risk", Resolution: "Rare enough", - RaisedBy: api.DecisionUser{Name: "Theo"}, - Author: api.DecisionAuthor{Role: "coding_agent", Model: "m", Harness: "claude-code"}, - Comments: []api.DecisionComment{{ - ID: 1, User: api.DecisionUser{Name: "Theo"}, Author: api.DecisionAuthor{Role: "person"}, Body: "hi", - }}, + RaisedBy: &api.DecisionUser{Name: "Theo"}, + Author: &api.DecisionAuthor{Role: "coding_agent", Model: "m", Harness: "claude-code"}, + Comments: []api.DecisionComment{ + {ID: 1, User: &api.DecisionUser{Name: "Theo"}, Author: &api.DecisionAuthor{Role: "person"}, Body: "hi"}, + {ID: 2, User: &api.DecisionUser{Name: "Theo"}, Body: "no author"}, + }, }) for _, want := range []string{ - "raised by Theo · last changed as coding_agent (m, claude-code)", + "risk · settled (accepted_risk) · raised by Theo · last changed as coding_agent (m, claude-code)\n", "Resolution:\n Rare enough", "Theo as person", } { @@ -184,4 +210,16 @@ func TestFormatDecisionDetail(t *testing.T) { t.Errorf("detail lacks %q:\n%s", want, got) } } + // A null comment author leaves out the role instead of printing it blank. + for _, line := range strings.Split(got, "\n") { + if strings.Contains(line, "#2") && strings.Contains(line, " as ") { + t.Errorf("comment with no author: %q", line) + } + } + + // raised_by and author can be null; they're left out rather than blank. + got = formatDecisionDetail(&api.Decision{ID: 7, Type: "risk", Title: "T", Status: "open"}) + if !strings.Contains(got, " risk · open\n") { + t.Errorf("null raised_by and author:\n%s", got) + } } diff --git a/cmd/aviator/experimental_test.go b/cmd/aviator/hidden_test.go similarity index 78% rename from cmd/aviator/experimental_test.go rename to cmd/aviator/hidden_test.go index e689360..eaaa55b 100644 --- a/cmd/aviator/experimental_test.go +++ b/cmd/aviator/hidden_test.go @@ -1,5 +1,3 @@ -//go:build experimental - package main import ( @@ -8,9 +6,9 @@ import ( "testing" ) -// TestExperimentalCommandsHidden keeps the experimental commands out of help +// TestDecisionsCommandHidden keeps the decisions command out of help // and shell completion, which Homebrew builds from `aviator completion`. -func TestExperimentalCommandsHidden(t *testing.T) { +func TestDecisionsCommandHidden(t *testing.T) { for _, args := range [][]string{{"--help"}, {"help"}, {"__complete", ""}} { var out bytes.Buffer rootCmd.SetOut(&out) diff --git a/internal/api/decisions.go b/internal/api/decisions.go index e583709..4ada07d 100644 --- a/internal/api/decisions.go +++ b/internal/api/decisions.go @@ -1,5 +1,3 @@ -//go:build experimental - package api import ( @@ -33,21 +31,21 @@ type DecisionAnchor struct { } // DecisionComment is one entry in a decision's thread. User is the account -// that wrote it; Author is the role it was written as. +// that wrote it; Author is the role it was written as. Either can be null. type DecisionComment struct { - ID int `json:"id"` - User DecisionUser `json:"user"` - Author DecisionAuthor `json:"author"` - Body string `json:"body"` - CreatedAt string `json:"created_at"` - ModifiedAt string `json:"modified_at"` + ID int `json:"id"` + User *DecisionUser `json:"user"` + Author *DecisionAuthor `json:"author"` + Body string `json:"body"` + CreatedAt string `json:"created_at"` + ModifiedAt string `json:"modified_at"` } // Decision is a question, possible bug, risk, or choice raised about a // runbook's pull request. Status is open or settled; Outcome, SettledAt, and // Resolution are set once it is settled. RaisedBy is the user who created it -// and never changes; Author is the author of the latest change. Comments are -// in creation order. +// and never changes; Author is the author of the latest change. Either can be +// null. Comments are in creation order. type Decision struct { ID int `json:"id"` Type string `json:"type"` @@ -58,8 +56,8 @@ type Decision struct { SettledAt string `json:"settled_at"` Resolution string `json:"resolution"` Anchor *DecisionAnchor `json:"anchor"` - RaisedBy DecisionUser `json:"raised_by"` - Author DecisionAuthor `json:"author"` + RaisedBy *DecisionUser `json:"raised_by"` + Author *DecisionAuthor `json:"author"` Comments []DecisionComment `json:"comments"` } diff --git a/internal/api/decisions_test.go b/internal/api/decisions_test.go index e0c2c47..8df960e 100644 --- a/internal/api/decisions_test.go +++ b/internal/api/decisions_test.go @@ -1,5 +1,3 @@ -//go:build experimental - package api import ( @@ -46,6 +44,17 @@ func TestListDecisions(t *testing.T) { } } +func TestDecisionNullAuthors(t *testing.T) { + var d Decision + if err := json.Unmarshal([]byte(`{"id":7,"raised_by":null,"author":null,`+ + `"comments":[{"id":1,"user":null,"author":null,"body":"b"}]}`), &d); err != nil { + t.Fatal(err) + } + if d.RaisedBy != nil || d.Author != nil || d.Comments[0].User != nil || d.Comments[0].Author != nil { + t.Errorf("decision = %+v", d) + } +} + // recordRequests serves reply to every request and records each one as // "METHOD path body". func recordRequests(t *testing.T, reply string) (*httptest.Server, *[]string) { diff --git a/internal/config/experimental.go b/internal/config/experimental.go deleted file mode 100644 index 3c53377..0000000 --- a/internal/config/experimental.go +++ /dev/null @@ -1,7 +0,0 @@ -//go:build experimental - -package config - -// Experimental reports whether this binary was built with the experimental -// tag. No release sets it; only local `just build-experimental` builds do. -const Experimental = true diff --git a/internal/config/experimental_off.go b/internal/config/experimental_off.go deleted file mode 100644 index 1fb136e..0000000 --- a/internal/config/experimental_off.go +++ /dev/null @@ -1,7 +0,0 @@ -//go:build !experimental - -package config - -// Experimental reports whether this binary was built with the experimental -// tag. No release sets it; only local `just build-experimental` builds do. -const Experimental = false diff --git a/justfile b/justfile index b1ae0a2..5b8a3ec 100644 --- a/justfile +++ b/justfile @@ -5,14 +5,9 @@ default: build: go build -o aviator ./cmd/aviator -# Build ./aviator with the experimental (internal-only) commands -build-experimental: - go build -tags experimental -o aviator ./cmd/aviator - -# Run all tests with vet, with and without the experimental tag +# Run all tests with vet test: go test --vet=all ./... - go test --vet=all -tags experimental ./... # Check formatting and run golangci-lint (golangci-lint must be installed) lint: From aaf156b6234e01d781b84a6e1b3a26563770291a Mon Sep 17 00:00:00 2001 From: tr-heuser Date: Wed, 28 Oct 2026 16:02:15 -0700 Subject: [PATCH 4/4] cut decisions down to list, show, and apply for the new API The backend now takes one atomic POST /decisions batch, so the per-verb commands (create, edit, comment, settle, reopen) are gone: each rebuilt part of the server's rules client-side and got some wrong. apply sends the file's bytes unchanged and lets the server's strict models validate them, so nulls, zero ids, and unknown keys are no longer dropped in a Go round trip. Comment edits move into the batch. list follows the paged API with --status, --page, and --limit and reads comment_count. The JSON list key is id instead of session, and list output leaves comments out rather than printing []. --- CLAUDE.md | 38 +-- cmd/aviator/decisions.go | 555 +++++--------------------------- cmd/aviator/decisions_format.go | 59 ++-- cmd/aviator/decisions_test.go | 213 ++++-------- internal/api/client.go | 5 + internal/api/decisions.go | 181 +++++------ internal/api/decisions_test.go | 101 ++---- 7 files changed, 296 insertions(+), 856 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 357b5a9..85212fc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,7 +29,7 @@ cmd/aviator/ # CLI entry point + commands (one file per command) evidence.go # `aviator evidence` -> signed URL / download of one evidence file invariants.go # `aviator invariants ` invariants_format.go # text rendering for the invariants commands - decisions.go # `aviator decisions ` (hidden) + decisions.go # `aviator decisions ` (hidden) decisions_format.go # text rendering for the decisions commands version.go # `aviator version` helpers.go # parseRepo(s), readSpecFile, collectCriteria, collectBody @@ -161,33 +161,15 @@ The CLI targets endpoints in the `mergeit` backend: redirect and the download is a plain unauthenticated GET. - `/api/v1/verify//decisions` — for `aviator decisions`, which is - `Hidden: true` while it changes (out of help and completion, but it runs - when called by name). Every body rejects unknown keys. - - `GET` — `{decisions: [...]}`, open first. `GET /` — one decision. - Reads take any API token. - - `POST /batch` — `{decisions: [{id?, author, type?, title?, summary?, - anchor?, outcome?, resolution?, comments?: [{author, body, - model_metadata?}]}]}`, all-or-nothing, decisions returned in request - order. No `id` creates (`type` and `title` required); an `id` changes only - the keys sent. `outcome` settles, `outcome: null` reopens (and clears - `resolution`), `anchor: null` clears the anchor. A settled decision - accepts only reopening and comments (409 otherwise). The same `id` can't - appear twice. The CLI models the three-state keys with `api.Nullable`. - - `author` is `{role, model?, harness?, skills?}` on every item and comment; - `role` is `person`, `coding_agent`, `review_agent`, or `external_bot`. The - CLI's `--role`, `--model`, `--harness`, and `--skill` fill it. - `model_metadata` is free-form, per comment, stored but not returned. - - No `via`: the server credits an `aviator login` token to `aviator-cli` and - a personal token to `api`. - - `PATCH //comments/` — `{body}`. - - Writes need a user token on an account with Verify. Only the user who - raised a decision can change it (403); anyone in the account can comment. - - Responses carry `raised_by` (the user who created it), `author` (of the - latest change), `resolution`, and per comment a `user` (the account) and - an `author` (the role). - - Every command sends one batch: `aviator decisions apply` sends its file - as the batch body, and the single-decision commands build one, an item per - decision ID. + `Hidden: true` while it changes. `GET` lists in id order without comments + (`comment_count` instead), `?status=open|settled&page=&per_page=`, with + `has_more`; `GET /` is one decision with its comments. `POST` takes + `{decisions: [...]}` and applies it all or nothing: no `id` creates, an `id` + changes only the keys sent, `outcome` settles (needs `resolution`), + `outcome: null` reopens (needs a comment), and a comment with an `id` edits + it. Anyone can settle, reopen, or comment; only the raiser can change type, + title, summary, or anchor. `aviator decisions apply` sends its file as the + body unchanged and the server's strict models validate it. `POST` and `PATCH /api/v1/verify`, dismissals, and the listing are gated on `role="user"`: an account-scoped API token resolves to no role and gets a 403, diff --git a/cmd/aviator/decisions.go b/cmd/aviator/decisions.go index a71de09..ca2e134 100644 --- a/cmd/aviator/decisions.go +++ b/cmd/aviator/decisions.go @@ -1,7 +1,6 @@ package main import ( - "encoding/json" "fmt" "io" "os" @@ -13,15 +12,6 @@ import ( "github.com/aviator-co/aviator-cli/internal/utils/colors" "github.com/dustin/go-humanize/english" "github.com/spf13/cobra" - "github.com/spf13/pflag" -) - -// decisionTypes, decisionOutcomes, and decisionRoles name the server's values -// for flag help. The server validates them, so nothing is checked client-side. -const ( - decisionTypes = "question, possible_bug, risk, choice" - decisionOutcomes = "fixed, intended, accepted_risk, follow_up, withdrawn, decided" - decisionRoles = "person, coding_agent, review_agent, external_bot" ) // decisionsFlags holds the flags every decisions subcommand shares. @@ -29,19 +19,6 @@ var decisionsFlags struct { JSON bool } -// authorFlags are the role a write is recorded as, and the agent details -// behind it when known. -type authorFlags struct { - Role string - Model string - Harness string - Skills []string -} - -func (f *authorFlags) author() api.DecisionAuthor { - return api.DecisionAuthor{Role: f.Role, Model: f.Model, Harness: f.Harness, Skills: f.Skills} -} - var decisionsCmd = &cobra.Command{ Use: "decisions", Aliases: []string{"decision"}, @@ -53,20 +30,20 @@ var decisionsCmd = &cobra.Command{ "session's pull request. Each one has a comment thread and is either open\n" + "or settled with an outcome.\n" + "\n" + - "Every write is recorded under a --role (" + decisionRoles + "),\n" + - "with --model, --harness, and --skill describing the agent when there is\n" + - "one. edit, comment, settle, and reopen take several decision IDs and apply\n" + - "to all of them, and apply records a whole JSON batch. Each command sends\n" + - "one request, so either all of it is recorded or none of it is.\n" + - "\n" + - "Writes act as the caller, so they need a user access token. Comments are\n" + - "credited to aviator-cli when you're signed in with `aviator login`, and\n" + - "to the API when you use a personal token.", + "list and show read them. apply records a JSON batch that creates,\n" + + "changes, settles, reopens, and comments on decisions in one request, so\n" + + "either all of it is recorded or none of it is.", +} + +var decisionsListFlags struct { + Status string + Page int + Limit int } var decisionsListCmd = &cobra.Command{ Use: "list ", - Short: "List a session's decisions, open ones first (e.g. aviator decisions list r/123)", + Short: "List a session's decisions (e.g. aviator decisions list r/123 --status open)", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { runbookNumber, err := parseRunbookID(args[0]) @@ -77,22 +54,45 @@ var decisionsListCmd = &cobra.Command{ if err != nil { return err } - resp, err := client.ListDecisions(cmd.Context(), runbookNumber) + f := &decisionsListFlags + resp, err := client.ListDecisions(cmd.Context(), runbookNumber, api.ListDecisionsParams{ + Status: f.Status, + Page: f.Page, + PerPage: f.Limit, + }) if err != nil { return err } if decisionsFlags.JSON { - return printJSON(newDecisionListJSON(runbookNumber, resp.Decisions)) + out := newDecisionListJSON(runbookNumber, resp.Decisions) + out.HasMore = &resp.HasMore + return printJSON(out) } if len(resp.Decisions) == 0 { - fmt.Printf("No decisions on %s.\n", formatRunbookID(runbookNumber)) + fmt.Println(noDecisionsMessage(runbookNumber)) return nil } fmt.Print(formatDecisionTable(resp.Decisions)) + if resp.HasMore { + fmt.Printf("\nSee more decisions --page %d\n", max(f.Page, 1)+1) + } return nil }, } +// noDecisionsMessage says what was listed, and past the last page says that +// instead, since "no decisions" there would be a lie. +func noDecisionsMessage(runbookNumber int) string { + if decisionsListFlags.Page > 1 { + return fmt.Sprintf("Nothing on page %d, the listing ends before it. Try a lower --page.", + decisionsListFlags.Page) + } + if status := decisionsListFlags.Status; status != "" { + return fmt.Sprintf("No %s decisions on %s.", status, formatRunbookID(runbookNumber)) + } + return fmt.Sprintf("No decisions on %s.", formatRunbookID(runbookNumber)) +} + var decisionsShowCmd = &cobra.Command{ Use: "show ", Short: "Show one decision with its comment thread", @@ -122,398 +122,67 @@ var decisionsShowCmd = &cobra.Command{ }, } -var decisionsCreateFlags struct { - Type string - Title string - Summary string - Path string - Lines string - Commit string - Body string - BodyFile string - Author authorFlags -} - -var decisionsCreateCmd = &cobra.Command{ - Use: "create ", - Short: "Raise a new decision on a session", - Long: "Raise a new decision on a session as --role. --body is the first\n" + - "comment on its thread, explaining why it was raised. --path, --lines,\n" + - "and --commit optionally pin it to lines of a file.", - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - f := &decisionsCreateFlags - body, err := collectBody(f.Body, f.BodyFile) - if err != nil { - return err - } - if body == "" { - return errors.New("--body (or --body-file) is required") - } - anchor, err := buildAnchor(nil, f.Path, f.Lines, f.Commit) - if err != nil { - return err - } - author := f.Author.author() - return recordDecisions(cmd, args[0], []api.DecisionBatchItem{{ - Author: author, - Type: f.Type, - Title: f.Title, - Summary: f.Summary, - Anchor: anchor, - Comments: []api.DecisionCommentInput{{Author: author, Body: body}}, - }}) - }, -} - -var decisionsEditFlags struct { - Type string - Title string - Summary string - Path string - Lines string - Commit string - Body string - BodyFile string - Author authorFlags -} - -var decisionsEditCmd = &cobra.Command{ - Use: "edit ...", - Short: "Change open decisions; every listed one gets the same changes", - Long: "Change the type, title, summary, or anchor of open decisions as\n" + - "--role. --path, --lines, and --commit change only those parts of each\n" + - "decision's current anchor. --body also adds a comment to each one's\n" + - "thread. Only the user who raised a decision can edit it, and settled\n" + - "decisions can't be edited.", - Args: cobra.MinimumNArgs(2), - RunE: func(cmd *cobra.Command, args []string) error { - f := &decisionsEditFlags - body, err := collectBody(f.Body, f.BodyFile) - if err != nil { - return err - } - anchorSet := f.Path != "" || f.Lines != "" || f.Commit != "" - if f.Type == "" && f.Title == "" && f.Summary == "" && !anchorSet && body == "" { - return errors.New("nothing to change: pass --type, --title, --summary, an anchor, or --body") - } - anchors := map[int]api.Nullable[api.DecisionAnchor]{} - if anchorSet { - if anchors, err = mergedAnchors(cmd, args, f.Path, f.Lines, f.Commit); err != nil { - return err - } - } - author := f.Author.author() - return recordForEach(cmd, args, func(id int) api.DecisionBatchItem { - item := api.DecisionBatchItem{ - ID: id, - Author: author, - Type: f.Type, - Title: f.Title, - Summary: f.Summary, - Anchor: anchors[id], - } - if body != "" { - item.Comments = []api.DecisionCommentInput{{Author: author, Body: body}} - } - return item - }) - }, -} - -var decisionsCommentFlags struct { - Body string - BodyFile string - Edit int - Author authorFlags -} - -var decisionsCommentCmd = &cobra.Command{ - Use: "comment ...", - Short: "Add the same comment to decisions, or edit one with --edit", - Long: "Add the same comment to each decision's thread as --role.\n" + - "\n" + - "--edit instead replaces the body of one of your comments on\n" + - "one decision (comment IDs are in `aviator decisions show`). Only the user\n" + - "who wrote a comment can edit it, and it keeps its author, so --edit takes\n" + - "no --role.", - Args: cobra.MinimumNArgs(2), - RunE: func(cmd *cobra.Command, args []string) error { - f := &decisionsCommentFlags - body, err := collectBody(f.Body, f.BodyFile) - if err != nil { - return err - } - if body == "" { - return errors.New("--body (or --body-file) is required") - } - if cmd.Flags().Changed("edit") { - return editDecisionComment(cmd, args, f.Edit, body) - } - if f.Author.Role == "" { - return errors.New("--role is required to add a comment") - } - author := f.Author.author() - return recordForEach(cmd, args, func(id int) api.DecisionBatchItem { - return api.DecisionBatchItem{ - ID: id, - Author: author, - Comments: []api.DecisionCommentInput{{Author: author, Body: body}}, - } - }) - }, -} - -var decisionsSettleFlags struct { - Outcome string - Summary string - Resolution string - Author authorFlags -} - -var decisionsSettleCmd = &cobra.Command{ - Use: "settle ...", - Short: "Settle open decisions with an outcome (e.g. aviator decisions settle r/123 7 8 --outcome fixed --resolution why --role person)", - Long: "Settle open decisions with an outcome (" + decisionOutcomes + ") as\n" + - "--role. --resolution says why it was settled this way, and --summary\n" + - "sets the decision's summary of what was decided in the same change. A\n" + - "settled decision can't be edited or settled again until it is reopened.", - Args: cobra.MinimumNArgs(2), - RunE: func(cmd *cobra.Command, args []string) error { - f := &decisionsSettleFlags - author := f.Author.author() - return recordForEach(cmd, args, func(id int) api.DecisionBatchItem { - return api.DecisionBatchItem{ - ID: id, - Author: author, - Summary: f.Summary, - Outcome: api.ValueOf(f.Outcome), - Resolution: strings.TrimSpace(f.Resolution), - } - }) - }, -} - -var decisionsReopenFlags struct { - Body string - BodyFile string - Author authorFlags -} - -var decisionsReopenCmd = &cobra.Command{ - Use: "reopen ...", - Short: "Reopen settled decisions", - Long: "Reopen settled decisions as --role, clearing their outcome and\n" + - "resolution. --body adds a comment saying why. Only the user who raised a\n" + - "decision can reopen it.", - Args: cobra.MinimumNArgs(2), - RunE: func(cmd *cobra.Command, args []string) error { - f := &decisionsReopenFlags - body, err := collectBody(f.Body, f.BodyFile) - if err != nil { - return err - } - author := f.Author.author() - return recordForEach(cmd, args, func(id int) api.DecisionBatchItem { - item := api.DecisionBatchItem{ID: id, Author: author, Outcome: api.Null[string]()} - if body != "" { - item.Comments = []api.DecisionCommentInput{{Author: author, Body: body}} - } - return item - }) - }, -} - var decisionsApplyFlags struct { File string } var decisionsApplyCmd = &cobra.Command{ Use: "apply ", - Short: "Create, edit, comment on, settle, and reopen decisions from a JSON batch file", + Short: "Create, change, settle, reopen, and comment on decisions from a JSON batch file", Long: "Record a batch of decisions from a JSON file (- reads stdin), all or\n" + - "nothing:\n" + + "nothing. The file is sent as is and the server checks it:\n" + "\n" + " {\"decisions\": [\n" + " {\"author\": {\"role\": \"coding_agent\", \"model\": \"claude-fable-5-1\",\n" + " \"harness\": \"claude-code\", \"skills\": [\"go-backend\"]},\n" + " \"type\": \"risk\", \"title\": \"...\", \"summary\": \"...\",\n" + " \"anchor\": {\"commit_sha\": \"abc1234\", \"file_path\": \"a.go\", \"start_line\": 10, \"end_line\": 20},\n" + - " \"comments\": [{\"author\": {\"role\": \"coding_agent\"}, \"body\": \"why it was raised\",\n" + - " \"model_metadata\": {\"subagent\": \"reviewer\"}}]},\n" + - " {\"id\": 7, \"author\": {\"role\": \"person\"},\n" + - " \"comments\": [{\"author\": {\"role\": \"person\"}, \"body\": \"a comment\"}]},\n" + + " \"comments\": [{\"body\": \"why it was raised\", \"model_metadata\": {\"subagent\": \"reviewer\"}}]},\n" + + " {\"id\": 7, \"comments\": [{\"author\": {\"role\": \"person\"}, \"body\": \"a comment\"},\n" + + " {\"id\": 23, \"body\": \"an edited comment\"}]},\n" + " {\"id\": 8, \"author\": {\"role\": \"person\"}, \"outcome\": \"fixed\", \"resolution\": \"why\"},\n" + - " {\"id\": 9, \"author\": {\"role\": \"person\"}, \"outcome\": null}\n" + + " {\"id\": 9, \"author\": {\"role\": \"person\"}, \"outcome\": null,\n" + + " \"comments\": [{\"body\": \"why it was reopened\"}]}\n" + " ]}\n" + "\n" + "An item without an id creates a decision and needs a type and title. One\n" + - "with an id changes only the keys it has: an outcome settles it, a null\n" + - "outcome reopens it, a null anchor clears its anchor, and comments alone\n" + - "only add comments. Every item and comment needs an author with a role\n" + - "(" + decisionRoles + "); add model, harness, and\n" + - "skills when you know them. Unknown keys are refused before anything is\n" + - "sent.", + "with an id changes only the keys it has: an outcome settles it and needs\n" + + "a resolution, a null outcome reopens it and needs a comment saying why,\n" + + "and null clears summary or anchor. An item that creates or changes a\n" + + "decision needs an author with a role (person, coding_agent,\n" + + "review_agent); add model, harness, and skills when you know them. A new\n" + + "comment without an author takes the item's. A comment with an id edits\n" + + "that comment and has only id and body.\n" + + "\n" + + "Anyone can settle, reopen, and comment. Only the user who raised a\n" + + "decision can change its type, title, summary, or anchor, and a settled\n" + + "decision has to be reopened first.", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - batch, err := readDecisionBatch(decisionsApplyFlags.File) + runbookNumber, err := parseRunbookID(args[0]) if err != nil { return err } - return recordDecisions(cmd, args[0], batch.Decisions) - }, -} - -// recordForEach builds one item per decision ID in args[1:] and records them -// as a single batch against the session in args[0]. -func recordForEach(cmd *cobra.Command, args []string, item func(id int) api.DecisionBatchItem) error { - items := make([]api.DecisionBatchItem, 0, len(args)-1) - for _, arg := range args[1:] { - id, err := parseDecisionID(arg) + body, err := readDecisionBatch(decisionsApplyFlags.File) if err != nil { return err } - items = append(items, item(id)) - } - return recordDecisions(cmd, args[0], items) -} - -// recordDecisions sends items to the session as one batch, then prints the -// decisions it touched as they now stand. -func recordDecisions(cmd *cobra.Command, sessionArg string, items []api.DecisionBatchItem) error { - runbookNumber, err := parseRunbookID(sessionArg) - if err != nil { - return err - } - client, err := api.NewClient() - if err != nil { - return err - } - resp, err := client.ApplyDecisions(cmd.Context(), runbookNumber, api.DecisionBatchRequest{Decisions: items}) - if err != nil { - return err - } - if decisionsFlags.JSON { - return printJSON(newDecisionListJSON(runbookNumber, resp.Decisions)) - } - fmt.Printf("%s %s: %s recorded\n", colors.Success("✓"), - formatRunbookID(runbookNumber), english.Plural(len(resp.Decisions), "decision", "")) - fmt.Print(formatDecisionTable(resp.Decisions)) - return nil -} - -// editDecisionComment replaces the body of comment commentID on the one -// decision in args[1]. -func editDecisionComment(cmd *cobra.Command, args []string, commentID int, body string) error { - if len(args) != 2 { - return errors.New("--edit takes exactly one decision ID") - } - if commentID <= 0 { - return errors.Errorf("invalid --edit %d, expected a comment ID", commentID) - } - runbookNumber, err := parseRunbookID(args[0]) - if err != nil { - return err - } - decisionID, err := parseDecisionID(args[1]) - if err != nil { - return err - } - client, err := api.NewClient() - if err != nil { - return err - } - c, err := client.EditDecisionComment(cmd.Context(), runbookNumber, decisionID, commentID, body) - if err != nil { - return err - } - if decisionsFlags.JSON { - return printJSON(newDecisionCommentJSON(c)) - } - fmt.Printf("%s %s: comment #%d on #%d edited\n", colors.Success("✓"), - formatRunbookID(runbookNumber), c.ID, decisionID) - fmt.Println(indent(strings.TrimSpace(c.Body), " ")) - return nil -} - -// mergedAnchors fetches the session in args[0] once and returns, for each -// decision ID in args[1:], its current anchor with the anchor flags laid over -// it. An ID the session doesn't have gets the flags alone; the batch then -// reports it as not found. -func mergedAnchors( - cmd *cobra.Command, args []string, path, lines, commit string, -) (map[int]api.Nullable[api.DecisionAnchor], error) { - runbookNumber, err := parseRunbookID(args[0]) - if err != nil { - return nil, err - } - client, err := api.NewClient() - if err != nil { - return nil, err - } - resp, err := client.ListDecisions(cmd.Context(), runbookNumber) - if err != nil { - return nil, errors.Wrap(err, "failed to fetch the current anchors") - } - current := make(map[int]*api.DecisionAnchor, len(resp.Decisions)) - for _, d := range resp.Decisions { - current[d.ID] = d.Anchor - } - anchors := make(map[int]api.Nullable[api.DecisionAnchor], len(args)-1) - for _, arg := range args[1:] { - id, err := parseDecisionID(arg) + client, err := api.NewClient() if err != nil { - return nil, err - } - if anchors[id], err = buildAnchor(current[id], path, lines, commit); err != nil { - return nil, errors.Wrapf(err, "#%d", id) + return err } - } - return anchors, nil -} - -// buildAnchor lays the anchor flags over base (nil when there is none), so a -// flag left out keeps base's value: the server replaces an anchor as a whole. -// The anchor is left out when no flags were passed. -func buildAnchor(base *api.DecisionAnchor, path, lines, commit string) (api.Nullable[api.DecisionAnchor], error) { - if path == "" && lines == "" && commit == "" { - return api.Nullable[api.DecisionAnchor]{}, nil - } - var anchor api.DecisionAnchor - if base != nil { - anchor = *base - } - if path != "" { - anchor.FilePath = path - } - if commit != "" { - anchor.CommitSHA = commit - } - if lines != "" { - start, end, err := parseLineRange(lines) + resp, err := client.ApplyDecisions(cmd.Context(), runbookNumber, body) if err != nil { - return api.Nullable[api.DecisionAnchor]{}, err + return err } - anchor.StartLine, anchor.EndLine = start, end - } - if anchor.StartLine > 0 && anchor.FilePath == "" { - return api.Nullable[api.DecisionAnchor]{}, errors.New("--lines needs --path") - } - return api.ValueOf(anchor), nil -} - -// parseLineRange reads --lines as or -. -func parseLineRange(s string) (start, end int, err error) { - first, last, isRange := strings.Cut(strings.TrimSpace(s), "-") - if !isRange { - last = first - } - start, errStart := strconv.Atoi(strings.TrimSpace(first)) - end, errEnd := strconv.Atoi(strings.TrimSpace(last)) - if errStart != nil || errEnd != nil || start < 1 || end < start { - return 0, 0, errors.Errorf("invalid --lines %q, expected or -", s) - } - return start, end, nil + if decisionsFlags.JSON { + return printJSON(newDecisionListJSON(runbookNumber, resp.Decisions)) + } + fmt.Printf("%s %s: %s recorded\n", colors.Success("✓"), + formatRunbookID(runbookNumber), english.Plural(len(resp.Decisions), "decision", "")) + fmt.Print(formatDecisionTable(resp.Decisions)) + return nil + }, } // parseDecisionID accepts a bare number or the # form the CLI prints. @@ -526,10 +195,8 @@ func parseDecisionID(arg string) (int, error) { return n, nil } -// readDecisionBatch reads apply's file, which is the API's batch body. Unknown -// keys are refused, so a typo, or a via or batch-level model_metadata from the -// old format, is caught before anything is sent. -func readDecisionBatch(path string) (*api.DecisionBatchRequest, error) { +// readDecisionBatch reads apply's file, or stdin for -, unparsed. +func readDecisionBatch(path string) ([]byte, error) { var data []byte var err error if path == "-" { @@ -540,84 +207,17 @@ func readDecisionBatch(path string) (*api.DecisionBatchRequest, error) { if err != nil { return nil, errors.Wrapf(err, "failed to read %s", path) } - dec := json.NewDecoder(strings.NewReader(string(data))) - dec.DisallowUnknownFields() - var req api.DecisionBatchRequest - if err := dec.Decode(&req); err != nil { - return nil, errors.Wrapf(err, "failed to parse %s", path) - } - if len(req.Decisions) == 0 { - return nil, errors.Errorf("%s has no decisions", path) - } - return &req, nil -} - -// registerAuthorFlags registers --role, --model, --harness, and --skill; -// required marks --role required, for commands where every use writes. -func registerAuthorFlags(cmd *cobra.Command, f *authorFlags, required bool) { - fs := cmd.Flags() - fs.StringVar(&f.Role, "role", "", "record this under a role ("+decisionRoles+")") - fs.StringVar(&f.Model, "model", "", "the model behind this, for an agent") - fs.StringVar(&f.Harness, "harness", "", "the harness the agent runs in (e.g. claude-code)") - fs.StringArrayVar(&f.Skills, "skill", nil, "a skill the agent used (repeatable)") - if required { - _ = cmd.MarkFlagRequired("role") - } -} - -func registerAnchorFlags(fs *pflag.FlagSet, path, lines, commit *string) { - fs.StringVar(path, "path", "", "pin to this file path") - fs.StringVar(lines, "lines", "", "pin to or - in --path") - fs.StringVar(commit, "commit", "", "the commit SHA the anchor refers to") + return data, nil } func init() { decisionsCmd.PersistentFlags().BoolVar(&decisionsFlags.JSON, "json", false, "print the result as a single JSON object instead of the human summary") - cf := decisionsCreateCmd.Flags() - cf.StringVar(&decisionsCreateFlags.Type, "type", "", "the kind of decision ("+decisionTypes+")") - cf.StringVar(&decisionsCreateFlags.Title, "title", "", "a one-line title") - cf.StringVar(&decisionsCreateFlags.Summary, "summary", "", "an optional longer description") - registerAnchorFlags(cf, &decisionsCreateFlags.Path, &decisionsCreateFlags.Lines, &decisionsCreateFlags.Commit) - registerBodyFlags(cf, &decisionsCreateFlags.Body, &decisionsCreateFlags.BodyFile, - "the first comment, explaining the decision") - registerAuthorFlags(decisionsCreateCmd, &decisionsCreateFlags.Author, true) - _ = decisionsCreateCmd.MarkFlagRequired("type") - _ = decisionsCreateCmd.MarkFlagRequired("title") - decisionsCreateCmd.MarkFlagsMutuallyExclusive("body", "body-file") - - ef := decisionsEditCmd.Flags() - ef.StringVar(&decisionsEditFlags.Type, "type", "", "change the kind of decision ("+decisionTypes+")") - ef.StringVar(&decisionsEditFlags.Title, "title", "", "change the title") - ef.StringVar(&decisionsEditFlags.Summary, "summary", "", "change the summary") - registerAnchorFlags(ef, &decisionsEditFlags.Path, &decisionsEditFlags.Lines, &decisionsEditFlags.Commit) - registerBodyFlags(ef, &decisionsEditFlags.Body, &decisionsEditFlags.BodyFile, - "also add this comment to each decision") - registerAuthorFlags(decisionsEditCmd, &decisionsEditFlags.Author, true) - decisionsEditCmd.MarkFlagsMutuallyExclusive("body", "body-file") - - registerBodyFlags(decisionsCommentCmd.Flags(), &decisionsCommentFlags.Body, &decisionsCommentFlags.BodyFile, - "the comment") - decisionsCommentCmd.Flags().IntVar(&decisionsCommentFlags.Edit, "edit", 0, - "replace the body of this comment instead of adding one") - registerAuthorFlags(decisionsCommentCmd, &decisionsCommentFlags.Author, false) - decisionsCommentCmd.MarkFlagsMutuallyExclusive("body", "body-file") - for _, name := range []string{"role", "model", "harness", "skill"} { - decisionsCommentCmd.MarkFlagsMutuallyExclusive("edit", name) - } - - registerBodyFlags(decisionsReopenCmd.Flags(), &decisionsReopenFlags.Body, &decisionsReopenFlags.BodyFile, - "add a comment saying why it was reopened") - registerAuthorFlags(decisionsReopenCmd, &decisionsReopenFlags.Author, true) - decisionsReopenCmd.MarkFlagsMutuallyExclusive("body", "body-file") - - sf := decisionsSettleCmd.Flags() - sf.StringVar(&decisionsSettleFlags.Outcome, "outcome", "", "how it was settled ("+decisionOutcomes+")") - sf.StringVar(&decisionsSettleFlags.Summary, "summary", "", "set the decision's summary of what was decided") - sf.StringVar(&decisionsSettleFlags.Resolution, "resolution", "", "why it was settled this way") - registerAuthorFlags(decisionsSettleCmd, &decisionsSettleFlags.Author, true) - _ = decisionsSettleCmd.MarkFlagRequired("outcome") + lf := decisionsListCmd.Flags() + lf.StringVar(&decisionsListFlags.Status, "status", "", "only open or settled decisions") + lf.IntVar(&decisionsListFlags.Page, "page", 1, "which page to show") + lf.IntVar(&decisionsListFlags.Limit, "limit", 20, "decisions per page (max 100)") decisionsApplyCmd.Flags().StringVar(&decisionsApplyFlags.File, "file", "", "the JSON batch file, or - for stdin") _ = decisionsApplyCmd.MarkFlagRequired("file") @@ -625,11 +225,6 @@ func init() { decisionsCmd.AddCommand( decisionsListCmd, decisionsShowCmd, - decisionsCreateCmd, - decisionsEditCmd, - decisionsCommentCmd, - decisionsSettleCmd, - decisionsReopenCmd, decisionsApplyCmd, ) rootCmd.AddCommand(decisionsCmd) diff --git a/cmd/aviator/decisions_format.go b/cmd/aviator/decisions_format.go index e9d0df7..5611678 100644 --- a/cmd/aviator/decisions_format.go +++ b/cmd/aviator/decisions_format.go @@ -18,7 +18,7 @@ func formatDecisionTable(decisions []api.Decision) string { fmt.Fprintln(w, "ID\tSTATUS\tTYPE\tCOMMENTS\tTITLE") for i := range decisions { d := &decisions[i] - fmt.Fprintf(w, "#%d\t%s\t%s\t%d\t%s\n", d.ID, formatDecisionStatus(d), d.Type, len(d.Comments), d.Title) + fmt.Fprintf(w, "#%d\t%s\t%s\t%d\t%s\n", d.ID, formatDecisionStatus(d), d.Type, d.NumComments(), d.Title) } _ = w.Flush() return b.String() @@ -144,23 +144,27 @@ type decisionCommentJSON struct { } type decisionJSON struct { - ID int `json:"id"` - Type string `json:"type"` - Title string `json:"title"` - Summary string `json:"summary"` - Status string `json:"status"` - Outcome string `json:"outcome"` - SettledAt string `json:"settled_at"` - Resolution string `json:"resolution"` - Anchor *decisionAnchorJSON `json:"anchor"` - RaisedBy *decisionUserJSON `json:"raised_by"` - Author *decisionAuthorJSON `json:"author"` - Comments []decisionCommentJSON `json:"comments"` + ID int `json:"id"` + Type string `json:"type"` + Title string `json:"title"` + Summary string `json:"summary"` + Status string `json:"status"` + Outcome string `json:"outcome"` + SettledAt string `json:"settled_at"` + Resolution string `json:"resolution"` + Anchor *decisionAnchorJSON `json:"anchor"` + RaisedBy *decisionUserJSON `json:"raised_by"` + Author *decisionAuthorJSON `json:"author"` + // Comments is left out of list, which doesn't carry the thread. + CommentCount int `json:"comment_count"` + Comments []decisionCommentJSON `json:"comments,omitzero"` } type decisionListJSON struct { - Session string `json:"session"` + ID string `json:"id"` Decisions []decisionJSON `json:"decisions"` + // HasMore is set by list, the one paged command. + HasMore *bool `json:"has_more,omitempty"` } func newDecisionUserJSON(u *api.DecisionUser) *decisionUserJSON { @@ -181,22 +185,25 @@ func newDecisionAuthorJSON(a *api.DecisionAuthor) *decisionAuthorJSON { func newDecisionJSON(d *api.Decision) decisionJSON { out := decisionJSON{ - ID: d.ID, - Type: d.Type, - Title: d.Title, - Summary: d.Summary, - Status: d.Status, - Outcome: d.Outcome, - SettledAt: d.SettledAt, - Resolution: d.Resolution, - RaisedBy: newDecisionUserJSON(d.RaisedBy), - Author: newDecisionAuthorJSON(d.Author), - Comments: make([]decisionCommentJSON, 0, len(d.Comments)), + ID: d.ID, + Type: d.Type, + Title: d.Title, + Summary: d.Summary, + Status: d.Status, + Outcome: d.Outcome, + SettledAt: d.SettledAt, + Resolution: d.Resolution, + RaisedBy: newDecisionUserJSON(d.RaisedBy), + Author: newDecisionAuthorJSON(d.Author), + CommentCount: d.NumComments(), } if d.Anchor != nil { anchor := decisionAnchorJSON(*d.Anchor) out.Anchor = &anchor } + if d.Comments != nil { + out.Comments = make([]decisionCommentJSON, 0, len(d.Comments)) + } for i := range d.Comments { out.Comments = append(out.Comments, newDecisionCommentJSON(&d.Comments[i])) } @@ -215,7 +222,7 @@ func newDecisionCommentJSON(c *api.DecisionComment) decisionCommentJSON { } func newDecisionListJSON(runbookNumber int, decisions []api.Decision) decisionListJSON { - out := decisionListJSON{Session: formatRunbookID(runbookNumber), Decisions: make([]decisionJSON, 0, len(decisions))} + out := decisionListJSON{ID: formatRunbookID(runbookNumber), Decisions: make([]decisionJSON, 0, len(decisions))} for i := range decisions { out.Decisions = append(out.Decisions, newDecisionJSON(&decisions[i])) } diff --git a/cmd/aviator/decisions_test.go b/cmd/aviator/decisions_test.go index 2f61cc4..44fce8a 100644 --- a/cmd/aviator/decisions_test.go +++ b/cmd/aviator/decisions_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/json" "io" "net/http" "net/http/httptest" @@ -15,167 +16,85 @@ import ( "github.com/spf13/cobra" ) -func TestParseLineRange(t *testing.T) { - for _, c := range []struct { - in string - start, end int - }{{"40", 40, 40}, {"40-52", 40, 52}, {" 3 - 4 ", 3, 4}} { - start, end, err := parseLineRange(c.in) - if err != nil || start != c.start || end != c.end { - t.Errorf("%q: %d-%d, %v", c.in, start, end, err) - } - } - for _, in := range []string{"", "x", "0", "52-40", "1-x"} { - if _, _, err := parseLineRange(in); err == nil { - t.Errorf("%q: want an error", in) - } - } -} - -func TestBuildAnchor(t *testing.T) { - base := &api.DecisionAnchor{FilePath: "a.go", StartLine: 40, EndLine: 52, CommitSHA: "abc"} - if a, err := buildAnchor(base, "", "", ""); !a.IsZero() || err != nil { - t.Errorf("no flags: %+v, %v", a, err) - } - if _, err := buildAnchor(nil, "", "4", ""); err == nil { - t.Error("--lines without --path: want an error") - } - for _, c := range []struct { - base *api.DecisionAnchor - path, lines, commit string - want api.DecisionAnchor - }{ - {nil, "a.go", "40-52", "abc", *base}, - // A flag changes only its part; the rest comes from the current anchor. - {base, "", "", "def", api.DecisionAnchor{FilePath: "a.go", StartLine: 40, EndLine: 52, CommitSHA: "def"}}, - {base, "", "7", "", api.DecisionAnchor{FilePath: "a.go", StartLine: 7, EndLine: 7, CommitSHA: "abc"}}, - {base, "b.go", "", "", api.DecisionAnchor{FilePath: "b.go", StartLine: 40, EndLine: 52, CommitSHA: "abc"}}, - } { - a, err := buildAnchor(c.base, c.path, c.lines, c.commit) - if err != nil || a.Value == nil || *a.Value != c.want { - t.Errorf("%+v: anchor = %+v, %v", c, a.Value, err) - } - } - if *base != (api.DecisionAnchor{FilePath: "a.go", StartLine: 40, EndLine: 52, CommitSHA: "abc"}) { - t.Errorf("base was changed: %+v", *base) - } -} - -// TestDecisionCommandBodies runs write commands against a test server and -// checks each sends its whole change as one batch. It calls RunE directly: -// going through rootCmd would load the real config and point the client away -// from the test server. -func TestDecisionCommandBodies(t *testing.T) { +// useTestServer points the API client at a server that records each request +// as "METHOD path?query body" and answers with reply. Commands are run through +// RunE directly: going through rootCmd would load the real config and point +// the client away from the test server. +func useTestServer(t *testing.T, reply string) *[]string { + t.Helper() var got []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { data, _ := io.ReadAll(r.Body) - got = append(got, r.Method+" "+r.URL.Path+" "+string(data)) - _, _ = w.Write([]byte(`{"decisions":[{"id":7,"status":"open",` + - `"anchor":{"commit_sha":"abc","file_path":"a.go","start_line":40,"end_line":52}},` + - `{"id":8,"status":"open","anchor":null}]}`)) + got = append(got, r.Method+" "+r.URL.RequestURI()+" "+string(data)) + _, _ = w.Write([]byte(reply)) })) - defer srv.Close() + t.Cleanup(srv.Close) host, token := config.Av.Aviator.APIHost, config.Av.Aviator.APIToken config.Av.Aviator.APIHost, config.Av.Aviator.APIToken = srv.URL, "test-token" t.Cleanup(func() { config.Av.Aviator.APIHost, config.Av.Aviator.APIToken = host, token }) - decisionsFlags.JSON = true - t.Cleanup(func() { decisionsFlags.JSON = false }) + return &got +} - for _, c := range []struct { - cmd *cobra.Command - args []string - want string - }{ - { - decisionsSettleCmd, - []string{ - "r/4", "7", "#8", "--outcome", "fixed", "--resolution", "why", - "--role", "coding_agent", "--model", "m", "--harness", "claude-code", "--skill", "a", "--skill", "b", - }, - `POST /api/v1/verify/4/decisions/batch {"decisions":[` + - `{"id":7,"author":{"role":"coding_agent","model":"m","harness":"claude-code","skills":["a","b"]},` + - `"outcome":"fixed","resolution":"why"},` + - `{"id":8,"author":{"role":"coding_agent","model":"m","harness":"claude-code","skills":["a","b"]},` + - `"outcome":"fixed","resolution":"why"}]}`, - }, - { - // The anchor flags are merged into each decision's current anchor, - // read in one GET, and the change is still one batch. - decisionsEditCmd, - []string{"r/4", "7", "8", "--commit", "def", "--role", "person"}, - "GET /api/v1/verify/4/decisions \n" + - `POST /api/v1/verify/4/decisions/batch {"decisions":[` + - `{"id":7,"author":{"role":"person"},"anchor":{"commit_sha":"def","file_path":"a.go","start_line":40,"end_line":52}},` + - `{"id":8,"author":{"role":"person"},"anchor":{"commit_sha":"def"}}]}`, - }, - { - decisionsReopenCmd, - []string{"r/4", "7", "--role", "person", "--body", "not fixed"}, - `POST /api/v1/verify/4/decisions/batch {"decisions":[{"id":7,"author":{"role":"person"},` + - `"outcome":null,"comments":[{"author":{"role":"person"},"body":"not fixed"}]}]}`, - }, - { - decisionsCreateCmd, - []string{ - "r/4", "--type", "risk", "--title", "T", "--body", "why", - "--path", "a.go", "--lines", "3", "--role", "review_agent", - }, - `POST /api/v1/verify/4/decisions/batch {"decisions":[{"author":{"role":"review_agent"},` + - `"type":"risk","title":"T","anchor":{"file_path":"a.go","start_line":3,"end_line":3},` + - `"comments":[{"author":{"role":"review_agent"},"body":"why"}]}]}`, - }, - } { - got = nil - c.cmd.SetContext(context.Background()) - if err := c.cmd.ParseFlags(c.args); err != nil { - t.Fatalf("%v: %v", c.args, err) - } - if err := c.cmd.RunE(c.cmd, c.cmd.Flags().Args()); err != nil { - t.Fatalf("%v: %v", c.args, err) - } - if strings.Join(got, "\n") != c.want { - t.Errorf("%v: requests =\n%s\nwant\n%s", c.args, strings.Join(got, "\n"), c.want) - } - } +func runCommand(t *testing.T, cmd *cobra.Command, args ...string) string { + t.Helper() + cmd.SetContext(context.Background()) + if err := cmd.ParseFlags(args); err != nil { + t.Fatalf("%v: %v", args, err) + } + stdout := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stdout = w + runErr := cmd.RunE(cmd, cmd.Flags().Args()) + os.Stdout = stdout + _ = w.Close() + out, _ := io.ReadAll(r) + if runErr != nil { + t.Fatalf("%v: %v", args, runErr) + } + return string(out) } -func TestReadDecisionBatch(t *testing.T) { - write := func(content string) string { - path := filepath.Join(t.TempDir(), "batch.json") - if err := os.WriteFile(path, []byte(content), 0o600); err != nil { - t.Fatal(err) - } - return path - } - req, err := readDecisionBatch(write(`{"decisions":[` + - `{"author":{"role":"coding_agent","model":"m"},"type":"risk","title":"T",` + - `"comments":[{"author":{"role":"coding_agent"},"body":"b","model_metadata":{"subagent":"r"}}]},` + - `{"id":8,"author":{"role":"person"},"outcome":"fixed","resolution":"why"},` + - `{"id":9,"author":{"role":"person"},"outcome":null,"anchor":null}]}`)) - if err != nil || len(req.Decisions) != 3 { - t.Fatalf("req = %+v, err = %v", req, err) +// TestDecisionsApplySendsFile checks apply sends the file unchanged, so the +// server sees the nulls, zero ids, and unknown keys a Go round trip drops. +func TestDecisionsApplySendsFile(t *testing.T) { + got := useTestServer(t, `{"decisions":[{"id":7,"status":"open","comments":[]}]}`) + body := `{"decisions": [{"id": 0, "summary": null, "anchor": {"file_path": "a.go", "line": 40}}]}` + "\n" + path := filepath.Join(t.TempDir(), "batch.json") + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + runCommand(t, decisionsApplyCmd, "r/4", "--file", path) + if want := "POST /api/v1/verify/4/decisions " + body; len(*got) != 1 || (*got)[0] != want { + t.Errorf("requests =\n%s\nwant\n%s", strings.Join(*got, "\n"), want) } - if d := req.Decisions[0]; d.Author.Model != "m" || d.Outcome.Set || d.Comments[0].ModelMetadata["subagent"] != "r" { - t.Errorf("create = %+v", d) +} + +func TestDecisionsListJSON(t *testing.T) { + got := useTestServer(t, `{"decisions":[{"id":7,"status":"open","comment_count":3}],`+ + `"page":2,"per_page":5,"has_more":true}`) + decisionsFlags.JSON = true + t.Cleanup(func() { + decisionsFlags.JSON = false + decisionsListFlags.Status, decisionsListFlags.Page, decisionsListFlags.Limit = "", 1, 20 + }) + out := runCommand(t, decisionsListCmd, "r/4", "--status", "open", "--page", "2", "--limit", "5") + if want := "GET /api/v1/verify/4/decisions?page=2&per_page=5&status=open "; (*got)[0] != want { + t.Errorf("request = %q, want %q", (*got)[0], want) } - if d := req.Decisions[1]; *d.Outcome.Value != "fixed" || d.Resolution != "why" { - t.Errorf("settle = %+v", d) + // The list carries no threads, so comments is left out rather than []. + var parsed map[string]any + if err := json.Unmarshal([]byte(out), &parsed); err != nil { + t.Fatalf("%v:\n%s", err, out) } - if d := req.Decisions[2]; !d.Outcome.Set || d.Outcome.Value != nil || !d.Anchor.Set || d.Anchor.Value != nil { - t.Errorf("reopen = %+v", d) + d := parsed["decisions"].([]any)[0].(map[string]any) + if parsed["id"] != "r/4" || parsed["has_more"] != true || d["comment_count"] != 3.0 || d["comments"] != nil { + t.Errorf("json = %s", out) } - for name, content := range map[string]string{ - "via": `{"via":"aviator-cli","decisions":[{"id":7,"author":{"role":"person"}}]}`, - "batch model_metadata": `{"model_metadata":{"model":"m"},"decisions":[{"id":7,"author":{"role":"person"}}]}`, - "unknown key": `{"decisions":[{"id":7,"author":{"role":"person"},"coments":[]}]}`, - "old nested format": `{"decisions":[{"decision":{"id":7}}]}`, - "author name": `{"decisions":[{"id":7,"author":{"role":"person","name":"Sam"}}]}`, - "bare array": `[{"id":7}]`, - "empty": `{"decisions":[]}`, - } { - if _, err := readDecisionBatch(write(content)); err == nil { - t.Errorf("%s: want an error", name) - } + if _, ok := parsed["session"]; ok { + t.Errorf("json still has session: %s", out) } } diff --git a/internal/api/client.go b/internal/api/client.go index b1896d5..f389856 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -81,7 +81,12 @@ func (c *Client) doJSON(ctx context.Context, method, path string, body, out any) return errors.Wrap(err, "failed to encode request") } } + return c.doPayload(ctx, method, path, payload, out) +} +// doPayload sends payload (when non-nil), already-encoded JSON, to path using +// method, and handles the response like doJSON. +func (c *Client) doPayload(ctx context.Context, method, path string, payload []byte, out any) error { status, _, data, err := c.do(ctx, c.http, method, path, payload) if err != nil { return err diff --git a/internal/api/decisions.go b/internal/api/decisions.go index 4ada07d..37cd1ab 100644 --- a/internal/api/decisions.go +++ b/internal/api/decisions.go @@ -2,13 +2,14 @@ package api import ( "context" - "encoding/json" "fmt" + "net/http" + "net/url" + "strconv" ) // DecisionAuthor is the role, and the agent details when known, behind a -// decision write or a comment. Role is person, coding_agent, review_agent, or -// external_bot. +// decision write or a comment. Role is person, coding_agent, or review_agent. type DecisionAuthor struct { Role string `json:"role"` Model string `json:"model,omitempty"` @@ -30,6 +31,12 @@ type DecisionAnchor struct { EndLine int `json:"end_line,omitempty"` } +// DecisionRepo is the repository a decision's pull request is in. +type DecisionRepo struct { + Org string `json:"org"` + Name string `json:"name"` +} + // DecisionComment is one entry in a decision's thread. User is the account // that wrote it; Author is the role it was written as. Either can be null. type DecisionComment struct { @@ -45,87 +52,69 @@ type DecisionComment struct { // runbook's pull request. Status is open or settled; Outcome, SettledAt, and // Resolution are set once it is settled. RaisedBy is the user who created it // and never changes; Author is the author of the latest change. Either can be -// null. Comments are in creation order. +// null. The list carries CommentCount and no Comments; one decision, and the +// decisions a POST returns, carry Comments in id order and no CommentCount. type Decision struct { - ID int `json:"id"` - Type string `json:"type"` - Title string `json:"title"` - Summary string `json:"summary"` - Status string `json:"status"` - Outcome string `json:"outcome"` - SettledAt string `json:"settled_at"` - Resolution string `json:"resolution"` - Anchor *DecisionAnchor `json:"anchor"` - RaisedBy *DecisionUser `json:"raised_by"` - Author *DecisionAuthor `json:"author"` - Comments []DecisionComment `json:"comments"` -} - -// DecisionBatchRequest is the body for POST -// /api/v1/verify//decisions/batch. The items are applied all-or-nothing. -type DecisionBatchRequest struct { - Decisions []DecisionBatchItem `json:"decisions"` -} - -// DecisionBatchItem creates a decision when ID is zero (Type and Title are -// required), and otherwise changes only the keys it sends. Author is credited -// with the creation or change. An Outcome settles the decision and a null one -// reopens it; a null Anchor clears it. -type DecisionBatchItem struct { - ID int `json:"id,omitempty"` - Author DecisionAuthor `json:"author,omitzero"` - Type string `json:"type,omitempty"` - Title string `json:"title,omitempty"` - Summary string `json:"summary,omitempty"` - Anchor Nullable[DecisionAnchor] `json:"anchor,omitzero"` - Outcome Nullable[string] `json:"outcome,omitzero"` - Resolution string `json:"resolution,omitempty"` - Comments []DecisionCommentInput `json:"comments,omitempty"` -} - -// DecisionCommentInput is one comment to add. ModelMetadata is free-form agent -// info the server stores but never returns. -type DecisionCommentInput struct { - Author DecisionAuthor `json:"author,omitzero"` - Body string `json:"body"` - ModelMetadata map[string]any `json:"model_metadata,omitempty"` + ID int `json:"id"` + Type string `json:"type"` + Title string `json:"title"` + Summary string `json:"summary"` + Status string `json:"status"` + Outcome string `json:"outcome"` + SettledAt string `json:"settled_at"` + Resolution string `json:"resolution"` + Anchor *DecisionAnchor `json:"anchor"` + Repo *DecisionRepo `json:"repo"` + RaisedBy *DecisionUser `json:"raised_by"` + Author *DecisionAuthor `json:"author"` + CreatedAt string `json:"created_at"` + ModifiedAt string `json:"modified_at"` + CommentCount int `json:"comment_count"` + Comments []DecisionComment `json:"comments"` +} + +// NumComments is the length of the thread, from Comments when the response +// carried them and from CommentCount when it didn't. +func (d *Decision) NumComments() int { + if d.Comments != nil { + return len(d.Comments) + } + return d.CommentCount } -// Nullable is a key that can be left out, set to a value, or sent as null. -// The zero value is left out (with the omitzero tag). -type Nullable[T any] struct { - Set bool - Value *T +// ListDecisionsParams filters and pages GET /api/v1/verify//decisions. +// Status is open or settled, blank for both. A zero Page or PerPage leaves it +// to the server (page 1, 20 per page, at most 100). +type ListDecisionsParams struct { + Status string + Page int + PerPage int } -// Null returns a Nullable that is sent as null. -func Null[T any]() Nullable[T] { return Nullable[T]{Set: true} } - -// ValueOf returns a Nullable that is sent as v. -func ValueOf[T any](v T) Nullable[T] { return Nullable[T]{Set: true, Value: &v} } - -// IsZero reports whether the key is left out. -func (n Nullable[T]) IsZero() bool { return !n.Set } - -// MarshalJSON writes the value, or null. -func (n Nullable[T]) MarshalJSON() ([]byte, error) { return json.Marshal(n.Value) } - -// UnmarshalJSON marks the key present, keeping null apart from a left-out key. -func (n *Nullable[T]) UnmarshalJSON(data []byte) error { - *n = Nullable[T]{Set: true} - return json.Unmarshal(data, &n.Value) -} - -// DecisionsResponse is the response from GET /api/v1/verify//decisions, -// open decisions first, and from the batch POST, in request order. -type DecisionsResponse struct { +// ListDecisionsResponse is one page of a runbook's decisions, in id order. +type ListDecisionsResponse struct { Decisions []Decision `json:"decisions"` -} - -// ListDecisions fetches a runbook's decisions. -func (c *Client) ListDecisions(ctx context.Context, runbookNumber int) (*DecisionsResponse, error) { - var out DecisionsResponse - if err := c.getJSON(ctx, decisionsPath(runbookNumber), nil, &out); err != nil { + Page int `json:"page"` + PerPage int `json:"per_page"` + HasMore bool `json:"has_more"` +} + +// ListDecisions fetches one page of a runbook's decisions, without comments. +func (c *Client) ListDecisions( + ctx context.Context, runbookNumber int, params ListDecisionsParams, +) (*ListDecisionsResponse, error) { + query := url.Values{} + if params.Status != "" { + query.Set("status", params.Status) + } + if params.Page > 0 { + query.Set("page", strconv.Itoa(params.Page)) + } + if params.PerPage > 0 { + query.Set("per_page", strconv.Itoa(params.PerPage)) + } + var out ListDecisionsResponse + if err := c.getJSON(ctx, decisionsPath(runbookNumber), query, &out); err != nil { return nil, err } return &out, nil @@ -140,13 +129,19 @@ func (c *Client) GetDecision(ctx context.Context, runbookNumber, decisionID int) return &out, nil } -// ApplyDecisions creates, changes, settles, and comments on decisions in one -// all-or-nothing batch, returning each item's decision in request order. -func (c *Client) ApplyDecisions( - ctx context.Context, runbookNumber int, req DecisionBatchRequest, -) (*DecisionsResponse, error) { - var out DecisionsResponse - if err := c.postJSON(ctx, decisionsPath(runbookNumber)+"/batch", req, &out); err != nil { +// ApplyDecisionsResponse is the decisions a POST touched, in request order, +// each with its comments. +type ApplyDecisionsResponse struct { + Decisions []Decision `json:"decisions"` +} + +// ApplyDecisions sends body, a {"decisions": [...]} batch, to POST +// /api/v1/verify//decisions as is. The server validates it strictly and +// applies it all or nothing; decoding and re-encoding it here would drop +// nulls, zero ids, and unknown keys before the server could see them. +func (c *Client) ApplyDecisions(ctx context.Context, runbookNumber int, body []byte) (*ApplyDecisionsResponse, error) { + var out ApplyDecisionsResponse + if err := c.doPayload(ctx, http.MethodPost, decisionsPath(runbookNumber), body, &out); err != nil { return nil, err } return &out, nil @@ -159,19 +154,3 @@ func decisionsPath(runbookNumber int) string { func decisionPath(runbookNumber, decisionID int) string { return fmt.Sprintf("%s/%d", decisionsPath(runbookNumber), decisionID) } - -// EditDecisionComment replaces a comment's body. Only the user who wrote the -// comment can edit it. -func (c *Client) EditDecisionComment( - ctx context.Context, runbookNumber, decisionID, commentID int, body string, -) (*DecisionComment, error) { - req := struct { - Body string `json:"body"` - }{Body: body} - var out DecisionComment - path := fmt.Sprintf("%s/comments/%d", decisionPath(runbookNumber, decisionID), commentID) - if err := c.patchJSON(ctx, path, req, &out); err != nil { - return nil, err - } - return &out, nil -} diff --git a/internal/api/decisions_test.go b/internal/api/decisions_test.go index 8df960e..ee0eef3 100644 --- a/internal/api/decisions_test.go +++ b/internal/api/decisions_test.go @@ -21,27 +21,27 @@ const decisionJSON = `{"id":7,"type":"risk","title":"Repos share one rate limit" `"created_at":"2026-10-21T00:00:00Z","modified_at":"2026-10-21T00:00:00Z"}` func TestListDecisions(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet || r.URL.Path != "/api/v1/verify/123/decisions" { - t.Errorf("%s %s", r.Method, r.URL.Path) - } - _, _ = w.Write([]byte(`{"decisions":[` + decisionJSON + `]}`)) - })) - defer srv.Close() - - resp, err := newTestClient(srv).ListDecisions(context.Background(), 123) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(resp.Decisions) != 1 { - t.Fatalf("decisions = %+v", resp.Decisions) + srv, got := recordRequests(t, `{"decisions":[{"id":7,"status":"open","comment_count":3}],`+ + `"page":2,"per_page":5,"has_more":true}`) + resp, err := newTestClient(srv).ListDecisions(context.Background(), 123, + ListDecisionsParams{Status: "open", Page: 2, PerPage: 5}) + if err != nil || len(resp.Decisions) != 1 || !resp.HasMore || resp.Page != 2 { + t.Fatalf("resp = %+v, err = %v", resp, err) } - d := resp.Decisions[0] - if d.ID != 7 || d.Outcome != "accepted_risk" || d.Resolution != "Rare enough to ship" || - d.Anchor.EndLine != 52 || d.RaisedBy.Name != "Theo" || d.Author.Harness != "claude-code" || - len(d.Comments) != 1 || d.Comments[0].User.Name != "Theo" || d.Comments[0].Author.Role != "person" { + if d := resp.Decisions[0]; d.NumComments() != 3 || d.Comments != nil { t.Errorf("decision = %+v", d) } + if want := "GET /api/v1/verify/123/decisions?page=2&per_page=5&status=open "; (*got)[0] != want { + t.Errorf("request = %q, want %q", (*got)[0], want) + } + + // Left-out params leave the defaults to the server. + if _, err := newTestClient(srv).ListDecisions(context.Background(), 123, ListDecisionsParams{}); err != nil { + t.Fatal(err) + } + if want := "GET /api/v1/verify/123/decisions "; (*got)[1] != want { + t.Errorf("request = %q, want %q", (*got)[1], want) + } } func TestDecisionNullAuthors(t *testing.T) { @@ -56,13 +56,13 @@ func TestDecisionNullAuthors(t *testing.T) { } // recordRequests serves reply to every request and records each one as -// "METHOD path body". +// "METHOD path?query body". func recordRequests(t *testing.T, reply string) (*httptest.Server, *[]string) { t.Helper() var got []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { data, _ := io.ReadAll(r.Body) - got = append(got, r.Method+" "+r.URL.Path+" "+string(data)) + got = append(got, r.Method+" "+r.URL.RequestURI()+" "+string(data)) _, _ = w.Write([]byte(reply)) })) t.Cleanup(srv.Close) @@ -80,63 +80,16 @@ func TestGetDecision(t *testing.T) { } } -// TestApplyDecisionsBody checks the batch body: left-out keys stay out, and a -// null outcome or anchor is sent as null. -func TestApplyDecisionsBody(t *testing.T) { +// TestApplyDecisions checks the body goes out byte for byte: nulls, a zero +// id, unknown keys, and whitespace are the server's to judge. +func TestApplyDecisions(t *testing.T) { srv, got := recordRequests(t, `{"decisions":[`+decisionJSON+`]}`) - agent := DecisionAuthor{Role: "coding_agent", Model: "m", Harness: "claude-code", Skills: []string{"go"}} - resp, err := newTestClient(srv).ApplyDecisions(context.Background(), 123, DecisionBatchRequest{ - Decisions: []DecisionBatchItem{ - { - Author: agent, Type: "choice", Title: "T", Summary: "S", - Anchor: ValueOf(DecisionAnchor{FilePath: "a.go", StartLine: 4, EndLine: 4}), - Outcome: ValueOf("decided"), - Comments: []DecisionCommentInput{{Author: agent, Body: "b", ModelMetadata: map[string]any{"subagent": "r"}}}, - }, - {ID: 7, Author: DecisionAuthor{Role: "person"}, Outcome: Null[string](), Anchor: Null[DecisionAnchor]()}, - {ID: 8, Author: DecisionAuthor{Role: "person"}, Outcome: ValueOf("fixed"), Resolution: "why"}, - }, - }) - if err != nil || len(resp.Decisions) != 1 { + body := `{"decisions": [{"id": 0, "summary": null, "anchor": {"file_path": "a.go", "line": 40}}]}` + "\n" + resp, err := newTestClient(srv).ApplyDecisions(context.Background(), 123, []byte(body)) + if err != nil || len(resp.Decisions) != 1 || resp.Decisions[0].NumComments() != 1 { t.Fatalf("resp = %+v, err = %v", resp, err) } - want := `POST /api/v1/verify/123/decisions/batch {"decisions":[` + - `{"author":{"role":"coding_agent","model":"m","harness":"claude-code","skills":["go"]},` + - `"type":"choice","title":"T","summary":"S","anchor":{"file_path":"a.go","start_line":4,"end_line":4},` + - `"outcome":"decided","comments":[{"author":{"role":"coding_agent","model":"m","harness":"claude-code",` + - `"skills":["go"]},"body":"b","model_metadata":{"subagent":"r"}}]},` + - `{"id":7,"author":{"role":"person"},"anchor":null,"outcome":null},` + - `{"id":8,"author":{"role":"person"},"outcome":"fixed","resolution":"why"}]}` - if (*got)[0] != want { - t.Errorf("request =\n%s\nwant\n%s", (*got)[0], want) - } -} - -func TestNullableUnmarshal(t *testing.T) { - for in, want := range map[string]Nullable[string]{ - `{}`: {}, - `{"outcome":null}`: Null[string](), - `{"outcome":"x"}`: ValueOf("x"), - } { - var item DecisionBatchItem - if err := json.Unmarshal([]byte(in), &item); err != nil { - t.Fatalf("%s: %v", in, err) - } - got := item.Outcome - if got.Set != want.Set || (got.Value == nil) != (want.Value == nil) || - (got.Value != nil && *got.Value != *want.Value) { - t.Errorf("%s: outcome = %+v, want %+v", in, got, want) - } - } -} - -func TestEditDecisionCommentBody(t *testing.T) { - srv, got := recordRequests(t, `{"id":23,"user":{"id":7,"name":"Theo"},"author":{"role":"person"},"body":"new"}`) - c, err := newTestClient(srv).EditDecisionComment(context.Background(), 123, 7, 23, "new") - if err != nil || c.ID != 23 || c.Body != "new" || c.User.Name != "Theo" { - t.Fatalf("comment = %+v, err = %v", c, err) - } - if want := `PATCH /api/v1/verify/123/decisions/7/comments/23 {"body":"new"}`; (*got)[0] != want { + if want := "POST /api/v1/verify/123/decisions " + body; (*got)[0] != want { t.Errorf("request =\n%s\nwant\n%s", (*got)[0], want) } }