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
12 changes: 12 additions & 0 deletions cmd/kata/conn_dropped_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//go:build !windows

package main

import (
"errors"
"syscall"
)

func connDroppedErrno(err error) bool {
return errors.Is(err, syscall.ECONNRESET) || errors.Is(err, syscall.EPIPE)
}
33 changes: 33 additions & 0 deletions cmd/kata/conn_dropped_unix_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//go:build !windows

package main

import (
"net"
"net/http"
"net/url"
"os"
"syscall"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestConnectionDroppedErrnosUnix(t *testing.T) {
for _, errno := range []syscall.Errno{syscall.EPIPE, syscall.ECONNRESET} {
err := &url.Error{Op: http.MethodPost, URL: "https://daemon.example/issues", Err: &net.OpError{
Op: "write", Net: "tcp", Err: errno,
}}
got := createRequestError(err, false)
var cliErr *cliError
require.ErrorAs(t, got, &cliErr)
assert.Equal(t, "create_outcome_unknown", cliErr.Code)
assert.NotContains(t, cliErr.Message, "daemon.example")
}

refused := &url.Error{Op: http.MethodPost, URL: "https://daemon.example/issues", Err: &net.OpError{
Op: "dial", Err: os.NewSyscallError("connect", syscall.ECONNREFUSED),
}}
assert.Same(t, refused, createRequestError(refused, false))
}
14 changes: 14 additions & 0 deletions cmd/kata/conn_dropped_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//go:build windows

package main

import (
"errors"
"syscall"
)

func connDroppedErrno(err error) bool {
return errors.Is(err, syscall.WSAECONNRESET) ||
errors.Is(err, syscall.WSAECONNABORTED) ||
errors.Is(err, syscall.ERROR_BROKEN_PIPE)
}
36 changes: 36 additions & 0 deletions cmd/kata/conn_dropped_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//go:build windows

package main

import (
"net"
"net/http"
"net/url"
"syscall"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestConnectionDroppedErrnosWindows(t *testing.T) {
for _, errno := range []syscall.Errno{
syscall.WSAECONNRESET,
syscall.WSAECONNABORTED,
syscall.ERROR_BROKEN_PIPE,
} {
err := &url.Error{Op: http.MethodPost, URL: "https://daemon.example/issues", Err: &net.OpError{
Op: "write", Net: "tcp", Err: errno,
}}
got := createRequestError(err, false)
var cliErr *cliError
require.ErrorAs(t, got, &cliErr)
assert.Equal(t, "create_outcome_unknown", cliErr.Code)
assert.NotContains(t, cliErr.Message, "daemon.example")
}

refused := &url.Error{Op: http.MethodPost, URL: "https://daemon.example/issues", Err: &net.OpError{
Op: "dial", Err: syscall.Errno(10061), // WSAECONNREFUSED
}}
assert.Same(t, refused, createRequestError(refused, false))
}
21 changes: 17 additions & 4 deletions cmd/kata/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ func createRequestError(err error, forceNew bool) error {
reason = "request timed out"
case errors.Is(err, context.Canceled):
reason = "request canceled before the response arrived"
case responseBodyWasCut(err):
reason = "the response was cut off before it completed"
case connectionDroppedBeforeResponse(err):
reason = "the connection dropped before the response arrived"
default:
return err
}
Expand All @@ -222,10 +226,19 @@ func requestTimedOut(err error) bool {
return true
}
var netErr net.Error
if !errors.As(err, &netErr) || netErr == nil {
return false
}
return netErr.Timeout()
// netErr != nil is guaranteed when errors.As returns true; the explicit
// check exists to satisfy NilAway (same idiom as internal/federation).
return errors.As(err, &netErr) && netErr != nil && netErr.Timeout()
}

func responseBodyWasCut(err error) bool {
var readErr *responseBodyReadError
return errors.As(err, &readErr)
}

func connectionDroppedBeforeResponse(err error) bool {
return errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) ||
connDroppedErrno(err)
}

// initialLinksAsChanges builds a synthetic mutationChanges from the
Expand Down
49 changes: 49 additions & 0 deletions cmd/kata/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ func TestCreateRequestError(t *testing.T) {
assert.Same(t, otherErr, createRequestError(otherErr, false))
}

func TestCreateRequestErrorConnectionDropped(t *testing.T) {
dropped := &url.Error{Op: http.MethodPost, URL: "https://daemon.example/issues", Err: io.EOF}
got := createRequestError(dropped, false)
var cliErr *cliError
require.ErrorAs(t, got, &cliErr)
assert.Equal(t, "create_outcome_unknown", cliErr.Code)
assert.Contains(t, cliErr.Message, "connection dropped")

}

func TestCreateRequestErrorCanceled(t *testing.T) {
canceledErr := &url.Error{
Op: http.MethodPost,
Expand Down Expand Up @@ -172,6 +182,45 @@ func TestCreateTimeoutClassificationAtCommandBoundary(t *testing.T) {
})
}

func TestCreateResponseBodyCutClassificationAtCommandBoundary(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/v1/projects/resolve":
w.Header().Set("Content-Type", "application/json")
_, _ = io.WriteString(w, `{"project":{"id":7,"name":"example-project"}}`)
case "/api/v1/projects/7/issues":
_, _ = io.Copy(io.Discard, r.Body)
hj, ok := w.(http.Hijacker)
if !ok {
http.Error(w, "hijacking unsupported", http.StatusInternalServerError)
return
}
conn, buf, err := hj.Hijack()
if err != nil {
return
}
defer func() { _ = conn.Close() }()
_, _ = buf.WriteString("HTTP/1.1 200 OK\r\n" +
"Content-Type: application/json\r\n" +
"Content-Length: 512\r\n\r\n" +
`{"issue":{"short_id":"abc4","title":"example issue","status":"open"`)
_ = buf.Flush()
default:
http.NotFound(w, r)
}
}))
t.Cleanup(server.Close)

ctx := contextWithBaseURL(context.Background(), server.URL)
_, _, err := executeRootCapture(t, ctx, "--workspace", t.TempDir(),
"create", "example issue")
cliErr := requireCLIError(t, err, ExitInternal)
assert.Equal(t, "create_outcome_unknown", cliErr.Code)
assert.Contains(t, cliErr.Message, "cut off")
assert.Contains(t, cliErr.Message, "check whether the issue was created")
assert.NotContains(t, cliErr.Message, "timed out")
}

func TestCreate_PrintsIssueShortIDInQuietMode(t *testing.T) {
env, dir := setupCLIEnv(t)
out := runCLI(t, env, dir, "--quiet", "create", "first issue", "--body", "details")
Expand Down
12 changes: 11 additions & 1 deletion cmd/kata/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,16 @@ func isShellSafeArg(s string) bool {
return true
}

// responseBodyReadError marks a response that was cut off after its headers
// arrived, when a mutating request may already have committed.
type responseBodyReadError struct{ err error }

func (e *responseBodyReadError) Error() string {
return "read response body: " + e.err.Error()
}

func (e *responseBodyReadError) Unwrap() error { return e.err }

// httpDoJSONWithHeader mirrors httpDoJSON but lets callers attach extra
// request headers (notably X-Kata-Confirm). Defined here so delete and the
// upcoming purge command don't have to extend the helpers.go signature.
Expand Down Expand Up @@ -274,7 +284,7 @@ func httpDoJSONWithHeader(ctx context.Context, client *http.Client,
defer func() { _ = resp.Body.Close() }()
out, err := io.ReadAll(resp.Body)
if err != nil {
return 0, nil, err
return 0, nil, &responseBodyReadError{err: err}
}
return resp.StatusCode, out, nil
}
9 changes: 5 additions & 4 deletions docs/reference/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,11 @@ kata create <title> \
`--meta` binds string-valued metadata at creation and is repeatable.

Before creating an issue, the daemon checks existing non-deleted look-alikes,
including closed ones, using the full title and the first 500 Unicode code
points of the body. `--force-new` bypasses that check; idempotency still wins
when an idempotency key matches. If create times out, or the request is
canceled before the response arrives, its outcome is unknown: check whether
including closed ones, using the first 500 Unicode code points of the title
and body. `--force-new` bypasses that check; idempotency still wins
when an idempotency key matches. If create times out, the request is canceled
or the connection drops before the response arrives, or the response is cut
off before it completes, its outcome is unknown: check whether
the issue exists before retrying, and use `--force-new` only after confirming
that no issue was created.

Expand Down
4 changes: 2 additions & 2 deletions internal/daemon/handlers_issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -1320,8 +1320,8 @@ func tryIdempotencyMatch(ctx context.Context, cfg ServerConfig, in *api.CreateIs
return fp, out, nil
}

// runLookalikeCheck runs the §3.7 soft-block: SearchFTSAny over the title and
// the body prefix used by the scorer
// runLookalikeCheck runs the §3.7 soft-block: SearchFTSAny over the
// 500-code-point title and body prefixes used by the scorer
// (OR-of-tokens for high recall), scores each candidate via similarity.Score,
// and returns a 409 duplicate_candidates error if any candidate is at or
// above the 0.7 threshold. nil means proceed. The OR variant is required
Expand Down
12 changes: 12 additions & 0 deletions internal/daemon/handlers_issues_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ func TestRunLookalikeCheckBoundsSearchQuery(t *testing.T) {
assert.Contains(t, store.query, "留")
assert.NotContains(t, store.query, "discard-this-suffix")
assert.True(t, utf8.ValidString(store.query))

store = &lookalikeQueryRecordingStore{}
in.Body.Title = strings.Repeat("界", 499) + "留 discarded-title-suffix"
in.Body.Body = "plain body"

err = runLookalikeCheck(context.Background(), ServerConfig{DB: store}, in)

require.NoError(t, err)
assert.Contains(t, store.query, "plain body")
assert.Contains(t, store.query, "留")
assert.NotContains(t, store.query, "discarded-title-suffix")
assert.True(t, utf8.ValidString(store.query))
}

type lookalikeCandidateStore struct {
Expand Down
5 changes: 4 additions & 1 deletion internal/hooks/dispatcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ func mustNewDispatcher(t *testing.T, hooks []ResolvedHook, cfg Config) (*Dispatc
// second wait matters on Windows, where an open appender prevents unlinking.
func cleanupDispatcher(t *testing.T, d *Dispatcher) {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
// Shutdown aborts in-flight work at deadline-2x GraceWindow, so a 2s
// budget is too tight for five sequential process spawns on Windows CI.
const budget = 20 * dispatcherTestGraceWindow
ctx, cancel := context.WithTimeout(context.Background(), budget)
err := d.Shutdown(ctx)
cancel()
if err == nil {
Expand Down
38 changes: 25 additions & 13 deletions internal/similarity/similarity.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,31 +126,43 @@ func Jaccard(a, b []string) float64 {

// Score returns the weighted similarity between two issues:
//
// 0.6 * Jaccard(title tokens) + 0.4 * Jaccard(body[:500] tokens)
// 0.6 * Jaccard(title[:500] tokens) + 0.4 * Jaccard(body[:500] tokens)
//
// Body slicing is rune-based: the first 500 Unicode codepoints of each body
// are tokenized. Spec §3.7.
// Field slicing is rune-based: the first 500 Unicode codepoints of each title
// and body are tokenized. Spec §3.7.
func Score(titleA, bodyA, titleB, bodyB string) float64 {
titleScore := Jaccard(Tokenize(titleA), Tokenize(titleB))
titleScore := Jaccard(Tokenize(TitlePrefix(titleA)), Tokenize(TitlePrefix(titleB)))
bodyScore := Jaccard(Tokenize(BodyPrefix(bodyA)), Tokenize(BodyPrefix(bodyB)))
return 0.6*titleScore + 0.4*bodyScore
}

// BodyPrefix returns the first 500 runes of body. If body has fewer than 500
// runes, it is returned unchanged.
func BodyPrefix(body string) string {
const lookalikePrefixRunes = 500

func fieldPrefix(value string) string {
count := 0
for i := range body {
if count == 500 {
return body[:i]
for i := range value {
if count == lookalikePrefixRunes {
return value[:i]
}
count++
}
return body
return value
}

// BodyPrefix returns the first 500 runes of body. If body has fewer than 500
// runes, it is returned unchanged.
func BodyPrefix(body string) string {
return fieldPrefix(body)
}

// TitlePrefix returns the first 500 runes of title. If title has fewer than
// 500 runes, it is returned unchanged.
func TitlePrefix(title string) string {
return fieldPrefix(title)
}

// LookalikeQuery returns the issue text used to retrieve look-alike
// candidates. Its body portion matches the prefix used by Score.
// candidates. Its title and body portions match the prefixes used by Score.
func LookalikeQuery(title, body string) string {
return strings.TrimSpace(title + " " + BodyPrefix(body))
return strings.TrimSpace(TitlePrefix(title) + " " + BodyPrefix(body))
}
18 changes: 18 additions & 0 deletions internal/similarity/similarity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ func TestLookalikeQuery(t *testing.T) {
assert.Equal(t, "full issue title "+prefix, got)
}

func TestLookalikeQuery_TitleIsBounded(t *testing.T) {
title := strings.Repeat("界", 499) + "留 discard-this-suffix"

got := similarity.LookalikeQuery(title, "")

assert.Contains(t, got, "留")
assert.NotContains(t, got, "discard-this-suffix")
assert.True(t, utf8.ValidString(got))
}

const epsilon = 1e-9

type tokenizeTestCase struct {
Expand Down Expand Up @@ -144,6 +154,14 @@ func TestScore_Body500CharLimit(t *testing.T) {
"divergence past 500 chars must not affect the score")
}

func TestScore_Title500RuneLimit(t *testing.T) {
prefix := strings.Repeat("x", 500)
assertScore(t, 1.0,
prefix+" alpha-divergent", "same body",
prefix+" beta-divergent", "same body",
"title divergence past 500 runes must not affect the score")
}

// TestTokenize_AllStopWordsAreFiltered guards against stopword/stem ordering
// regressions. Spec §3.7: stopword removal must come BEFORE stemming so
// "has" doesn't stem to "ha" and slip through.
Expand Down
Loading