Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,41 @@ components:
- issue_uid
- at
type: object
CloseActionRequestBody:
additionalProperties: false
properties:
actor:
type: string
dry_run:
type: boolean
evidence:
items:
$ref: "#/components/schemas/Evidence"
type:
- array
- "null"
message:
type: string
reason:
enum:
- done
- wontfix
- duplicate
- superseded
- audit-no-change
- ""
type: string
retry_protocol:
enum:
- close-v1
- ""
type: string
source:
enum:
- tui
- ""
type: string
type: object
Comment:
additionalProperties: true
properties:
Expand Down Expand Up @@ -4595,7 +4630,7 @@ components:
type: object
info:
title: kata
version: 0.14.0
version: 0.15.0
openapi: 3.1.0
paths:
/api/v1/audit/closes:
Expand Down Expand Up @@ -6275,11 +6310,19 @@ paths:
required: true
schema:
type: string
- in: header
name: Idempotency-Key
schema:
type: string
- in: header
name: If-Match
schema:
type: string
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/ActionRequestBody"
$ref: "#/components/schemas/CloseActionRequestBody"
required: true
responses:
"200":
Expand Down
40 changes: 40 additions & 0 deletions cmd/kata/api_compat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"sync/atomic"
Expand Down Expand Up @@ -86,6 +87,45 @@ func TestFilteredListAllRejectsDaemonBeforeGlobalListFilters(t *testing.T) {
assert.Zero(t, listCalls.Load(), "the unfiltered old endpoint must not be queried")
}

func TestCloseRetryFlagsMakeOldDaemonRejectCloseBeforeMutation(t *testing.T) {
var closeCalls, mutations atomic.Int32
var retryProtocol string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/v1/projects/resolve":
_, _ = w.Write([]byte(`{"project":{"id":1,"name":"example-project"}}`))
case "/api/v1/projects/1/issues/abc1/actions/close":
closeCalls.Add(1)
var body map[string]any
require.NoError(t, json.NewDecoder(r.Body).Decode(&body))
retryProtocol, _ = body["retry_protocol"].(string)
if retryProtocol != "" {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"status":400,"error":{"code":"validation","message":"retry_protocol: unexpected property"}}`))
return
}
mutations.Add(1)
_, _ = w.Write([]byte(`{"changed":true}`))
default:
http.NotFound(w, r)
}
}))
t.Cleanup(server.Close)

_, _, err := executeRootCapture(t,
contextWithBaseURL(context.Background(), server.URL),
"--project", "example-project", "close", "abc1",
"--wontfix",
"--message", "Reviewed the request and recorded why the work should stop here.",
"--idempotency-key", "close-request-1")
require.Error(t, err)
assert.Contains(t, err.Error(), "retry_protocol")
assert.Equal(t, "close-v1", retryProtocol)
assert.Equal(t, int32(1), closeCalls.Load())
assert.Zero(t, mutations.Load(), "the legacy request schema must reject before mutation")
}

func TestListAllDefaultsToUnlimited(t *testing.T) {
var sentLimit atomic.Bool
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down
71 changes: 63 additions & 8 deletions cmd/kata/close.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"encoding/json"
"fmt"
"maps"
"net/http"
Expand All @@ -14,10 +15,12 @@ import (

func newCloseCmd() *cobra.Command {
var (
reason string
message string
evidence []string
dryRun bool
reason string
message string
evidence []string
dryRun bool
idempotencyKey string
ifMatch string

sugarDone bool
sugarWontfix bool
Expand Down Expand Up @@ -50,6 +53,16 @@ Instead, label and comment:
kata comment <ref> --body "what was attempted, what remains"`,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
if cmd.Flags().Changed("idempotency-key") && strings.TrimSpace(idempotencyKey) == "" {
return &cliError{
Message: "--idempotency-key must not be blank",
Kind: kindValidation,
ExitCode: ExitValidation,
}
}
if err := validateMetaIfMatchFlag(cmd, ifMatch); err != nil {
return err
}
// Resolve sugar -> reason (with conflict checks). Multiple sugar
// flags are mutually exclusive: a sequential switch would silently
// keep the first match and drop the rest (`--done --wontfix` would
Expand Down Expand Up @@ -127,7 +140,18 @@ Instead, label and comment:
if dryRun && currentOutputMode() == outputHuman && !flags.Quiet {
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), "close: dry-run (no mutations will occur)")
}
return runAction(cmd, args[0], "close", extra)
headers := map[string]string{}
if idempotencyKey != "" {
headers["Idempotency-Key"] = idempotencyKey
}
if strings.TrimSpace(ifMatch) != "" {
etag, err := normalizeMetaIfMatch(ifMatch)
if err != nil {
return err
}
headers["If-Match"] = etag
}
return runActionWithHeaders(cmd, args[0], "close", extra, headers)
},
}
cmd.Flags().StringVar(&reason, "reason", "",
Expand All @@ -144,6 +168,10 @@ Instead, label and comment:
"duplicate-of:<N>, superseded-by:<N>")
cmd.Flags().BoolVar(&dryRun, "dry-run", false,
"validate without mutating; reports the would-be close event")
cmd.Flags().StringVar(&idempotencyKey, "idempotency-key", "",
"send Idempotency-Key header for safe retry")
cmd.Flags().StringVar(&ifMatch, "if-match", "",
"expected issue revision (N or rev-N)")

cmd.Flags().BoolVar(&sugarDone, "done", false, "sugar for --reason done")
cmd.Flags().BoolVar(&sugarWontfix, "wontfix", false, "sugar for --reason wontfix")
Expand Down Expand Up @@ -213,6 +241,12 @@ func parseEvidenceFlags(raw []string) ([]api.Evidence, error) {
// the daemon action endpoint. If --comment was passed on the command, the
// comment is appended in a separate POST after the action succeeds.
func runAction(cmd *cobra.Command, raw, action string, extra map[string]any) error {
return runActionWithHeaders(cmd, raw, action, extra, nil)
}

func runActionWithHeaders(
cmd *cobra.Command, raw, action string, extra map[string]any, headers map[string]string,
) error {
comment, err := commentFromFlag(cmd)
if err != nil {
return err
Expand All @@ -224,20 +258,41 @@ func runAction(cmd *cobra.Command, raw, action string, extra map[string]any) err
actor, _ := resolveActor(ctx, flags.As, nil)
body := map[string]any{"actor": actor}
maps.Copy(body, extra)
if action == "close" && len(headers) > 0 {
body["retry_protocol"] = api.CloseRetryProtocol
}
client, err := httpClientFor(ctx, baseURL)
if err != nil {
return err
}
status, bs, err := httpDoJSON(ctx, client, http.MethodPost,
status, bs, err := httpDoJSONHeaders(ctx, client, http.MethodPost,
fmt.Sprintf("%s/api/v1/projects/%d/issues/%s/actions/%s", baseURL, pid, url.PathEscape(issue.RefForAPI), action),
body)
body, headers)
if err != nil {
return err
}
if status >= 400 {
return apiErrFromBody(status, bs)
}
if err := postFollowupComment(ctx, client, baseURL, pid, issue.RefForAPI, actor, comment); err != nil {
commentKey := ""
if key := headers["Idempotency-Key"]; key != "" {
commentKey = "close-comment:" + key
}
commentProjectID, commentIssueRef := pid, issue.RefForAPI
if comment != "" && commentKey != "" {
var response api.MutationResponse
if err := json.Unmarshal(bs, &response.Body); err != nil {
return fmt.Errorf("decode close response for follow-up comment: %w", err)
}
if response.Body.Issue.ProjectID <= 0 || response.Body.Issue.UID == "" {
return fmt.Errorf("close response is missing the issue identity for the follow-up comment")
}
commentProjectID = response.Body.Issue.ProjectID
commentIssueRef = response.Body.Issue.UID
}
if err := postFollowupCommentWithKey(
ctx, client, baseURL, commentProjectID, commentIssueRef, actor, comment, commentKey,
); err != nil {
return err
}
return printMutation(cmd, bs)
Expand Down
93 changes: 93 additions & 0 deletions cmd/kata/close_reopen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.kenn.io/kata/internal/db"
)

func TestCloseReopen_RoundTrip(t *testing.T) {
Expand Down Expand Up @@ -57,6 +58,51 @@ func TestClose_AgentOutputExternalEvidence(t *testing.T) {
assert.Contains(t, audit, `"evidence_types":["external"]`)
}

func TestCloseCmd_RetryFlagsReplayOriginalReceipt(t *testing.T) {
env, dir, _, ref := setupWorkspaceWithIssue(t, "test issue")
args := []string{
"--json", "close", ref,
"--done",
"--message", "Implemented the requested behavior and ran the focused tests.",
"--test", "go test ./cmd/kata",
"--idempotency-key", "close-request-1",
"--if-match", "1",
}
first := runCLI(t, env, dir, args...)
assert.Contains(t, first, `"changed":true`)

second := runCLI(t, env, dir, args...)
assert.Contains(t, second, `"changed":false`)
assert.Contains(t, second, `"reused":true`)
assert.Contains(t, second, `"original_event":`)
}

func TestCloseCmd_RejectsBlankIfMatch(t *testing.T) {
env, dir, _, ref := setupWorkspaceWithIssue(t, "test issue")
_, stderr, err := runCLIWithErr(t, env, dir,
"close", ref,
"--done",
"--message", "Implemented the requested behavior and ran the focused tests.",
"--test", "go test ./cmd/kata",
"--if-match", "")
require.Error(t, err)
assert.Contains(t, stderr, "--if-match must not be blank")
}

func TestCloseCmd_RejectsBlankIdempotencyKey(t *testing.T) {
env, dir, _, ref := setupWorkspaceWithIssue(t, "test issue")
_, stderr, err := runCLIWithErr(t, env, dir,
"close", ref,
"--wontfix",
"--message", "Reviewed the request and recorded why the work should stop here.",
"--idempotency-key", " ")
require.Error(t, err)
assert.Contains(t, stderr, "--idempotency-key must not be blank")

show := runCLI(t, env, dir, "show", ref, "--json")
assert.Contains(t, show, `"status":"open"`)
}

func TestClose_AgentDryRunSuppressesHumanBanner(t *testing.T) {
env, dir, _, ref := setupWorkspaceWithIssue(t, "test issue")

Expand Down Expand Up @@ -404,6 +450,53 @@ func TestClose_WithComment_AppendsComment(t *testing.T) {
assert.Equal(t, "closed", got.Issue.Status)
}

func TestClose_RetryWithCommentAppendsOneComment(t *testing.T) {
env, dir, pid, ref := setupWorkspaceWithIssue(t, "test issue")
args := []string{
"close", ref,
"--done",
"--message", "Implemented the requested behavior and ran the focused tests.",
"--test", "go test ./cmd/kata",
"--comment", "fixed in abc1234",
"--idempotency-key", "close-request-with-comment-1",
}
runCLI(t, env, dir, args...)
runCLI(t, env, dir, args...)

got := fetchIssueViaHTTPWithComments(t, env, pid, ref)
require.Len(t, got.Comments, 1)
assert.Equal(t, "fixed in abc1234", got.Comments[0].Body)
}

func TestClose_RetryWithCommentAfterMoveAppendsOneComment(t *testing.T) {
env, dir, sourceProjectID, ref := setupWorkspaceWithIssue(t, "test issue")
args := []string{
"close", ref,
"--done",
"--message", "Implemented the requested behavior and ran the focused tests.",
"--test", "go test ./cmd/kata",
"--comment", "fixed in abc1234",
"--idempotency-key", "close-request-with-comment-then-move",
}
runCLI(t, env, dir, args...)
issue, err := env.DB.IssueByShortID(t.Context(), sourceProjectID, ref, db.IncludeDeletedNo)
require.NoError(t, err)
target, err := env.DB.CreateProject(t.Context(), "target-project")
require.NoError(t, err)
_, err = env.DB.MoveIssueProject(t.Context(), db.MoveIssueProjectIn{
IssueID: issue.ID, FromProjectID: sourceProjectID, ToProjectID: target.ID,
IfMatchRev: issue.Revision, Actor: "coordinator",
})
require.NoError(t, err)

runCLI(t, env, dir, args...)

comments, err := env.DB.CommentsByIssue(t.Context(), issue.ID)
require.NoError(t, err)
require.Len(t, comments, 1)
assert.Equal(t, "fixed in abc1234", comments[0].Body)
}

func TestReopen_WithComment_AppendsComment(t *testing.T) {
env, dir, pid, ref := setupWorkspaceWithIssue(t, "test issue")
runCLI(t, env, dir, "close", ref,
Expand Down
Loading
Loading