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
60 changes: 60 additions & 0 deletions docs/bug-fixes.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,66 @@ Each bug fix entry should include:

## Bug Fixes

### 2026-07-22: Reachable infinite-loop vulnerability in `golang.org/x/text`

**Description:**
The `govulncheck` gate on PR #76 reported GO-2026-5970, an infinite loop on invalid input in `golang.org/x/text`. Itemize resolved `v0.37.0`, and the scanner found a reachable path through the CLI's Sentry-backed telemetry dependencies.

**Test Case:**
```bash
go run golang.org/x/vuln/cmd/govulncheck@v1.3.0 ./...
# Before: reports reachable GO-2026-5970 and exits nonzero.
# After: reports zero reachable vulnerabilities.
```

**Root Cause:**
The module graph selected `golang.org/x/text v0.37.0`, which predates the upstream fix in `v0.39.0`.

**Fix Applied:**
Upgraded the indirect `golang.org/x/text` dependency to `v0.39.0`.

**Verification:**
- The same `govulncheck` command that failed in CI passes locally.
- Full pre-commit and race suites pass.

---

### 2026-07-22: Transient Walmart and Monarch timeouts caused avoidable order failures

**Description:**
A production Walmart sync hit one 30-second Monarch update timeout and one 30-second Walmart ledger timeout. The Monarch multi-delivery consolidation stopped after its first idempotent update attempt. The Walmart handler then triggered a second ledger request while checking refunds; that request succeeded, but the original charge lookup had already returned an error, so the order fell back to its summary total and failed to match. The CLI nevertheless ended with `Sync completed successfully.` while reporting two errors.

**Test Case:**
```go
// internal/adapters/providers/walmart/order_multi_delivery_test.go:
// TestOrder_GetFinalCharges/retries_a_transient_ledger_timeout

// internal/application/sync/consolidator_test.go:
// TestConsolidator_ConsolidateTransactions/retries_a_transient_primary_update_timeout
// TestConsolidator_ConsolidateTransactions/resumed_consolidation_preserves_the_original_charge_note

// internal/application/sync/handlers/walmart_test.go:
// TestWalmartHandler_ProcessOrder_MultiDelivery_ResumesInterruptedConsolidation

// internal/cli/output_test.go:
// TestPrintSyncSummaryDoesNotReportSuccessWhenOrdersFailed
```

**Root Cause:**
The Walmart adapter relied on the upstream client's retry policy, which retries rate limits but not HTTP client timeouts. Consolidation made only one Monarch update attempt even though it sets the same amount and note on every attempt. Summary wording depended only on whether at least one order was processed, not whether any other order failed.

**Fix Applied:**
Walmart ledger reads and the idempotent Monarch consolidation update now retry once for network timeouts, while respecting caller cancellation and returning permanent errors immediately. Every Monarch update attempt remains in the API audit log. If an update reached Monarch but its response timed out, a later run recognizes the exact original consolidation note, finds any still-posted component transactions, preserves the note, and finishes deleting those extras before applying splits. Production summaries now say `Sync completed with N errors.` whenever the result contains errors and reserve the success message for error-free runs.

**Verification:**
- All five regression tests failed before the fix and pass after it.
- `go test ./...` passes.
- A rebuilt live dry-run recognized the interrupted `$70.28` consolidation and identified its still-posted `$64.22` component transaction.
- The authorized production repair preserved the `$70.28` primary, deleted its `$64.22` leftover, and applied two splits. It also consolidated the `$75.66` order from `$6.16 + $9.51 + $58.99`, deleted both extras, and applied two splits.
- Direct Monarch read-back returned both primary amounts, notes, and two splits each; all three deleted component IDs returned `ErrNotFound`.

---

### 2026-07-16: Completed Walmart in-store receipts were treated as payment pending

**Description:**
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ require (
golang.org/x/net v0.55.0 // indirect
golang.org/x/sync v0.21.0 // indirect
golang.org/x/sys v0.45.0 // indirect
golang.org/x/text v0.37.0 // indirect
golang.org/x/text v0.39.0 // indirect
modernc.org/libc v1.73.4 // indirect
modernc.org/mathutil v1.7.1 // indirect
modernc.org/memory v1.11.0 // indirect
Expand Down
12 changes: 6 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN8
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4=
golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ=
golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ=
golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20210916014120-12bc252f5db8/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
Expand Down Expand Up @@ -106,13 +106,13 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc=
golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38=
golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus=
golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/tools v0.45.0 h1:18qN3FAooORvApf5XjCXgsuayZOEtXf6JK18I3+ONa8=
golang.org/x/tools v0.45.0/go.mod h1:LuUGqqaXcXMEFEruIVJVm5mgDD8vww/z/SR1gQ4uE/0=
golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q=
golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
Expand Down
36 changes: 31 additions & 5 deletions internal/adapters/providers/walmart/order.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package walmart

import (
"context"
"errors"
"fmt"
"log/slog"
"net"
"time"

"github.com/eshaffer321/itemize/internal/adapters/providers"
Expand Down Expand Up @@ -314,13 +316,37 @@ func (o *Order) getLedger() (*walmartclient.OrderLedger, error) {
ctx = context.Background()
}

ledger, err := o.client.GetOrderLedger(ctx, o.GetID())
if err != nil {
return nil, fmt.Errorf("failed to get order ledger: %w", err)
const maxAttempts = 2
var lastErr error
for attempt := 1; attempt <= maxAttempts; attempt++ {
ledger, err := o.client.GetOrderLedger(ctx, o.GetID())
if err == nil {
o.ledgerCache = ledger
return ledger, nil
}

lastErr = err
if attempt == maxAttempts || !isRetryableLedgerError(ctx, err) {
break
}
if o.logger != nil {
o.logger.Warn("transient ledger request failed; retrying",
"order_id", o.GetID(),
"attempt", attempt,
"error", err)
}
}

return nil, fmt.Errorf("failed to get order ledger: %w", lastErr)
}

func isRetryableLedgerError(ctx context.Context, err error) bool {
if ctx.Err() != nil || errors.Is(err, context.Canceled) {
return false
}

o.ledgerCache = ledger
return ledger, nil
var networkErr net.Error
return errors.As(err, &networkErr) && networkErr.Timeout()
}

// IsMultiDelivery checks if order was split into multiple deliveries
Expand Down
35 changes: 35 additions & 0 deletions internal/adapters/providers/walmart/order_multi_delivery_test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,30 @@
package walmart

import (
"context"
"io"
"log/slog"
"net/url"
"testing"

walmartclient "github.com/eshaffer321/walmart-client-go/v2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type flakyLedgerClient struct {
calls int
ledger *walmartclient.OrderLedger
}

func (c *flakyLedgerClient) GetOrderLedger(_ context.Context, _ string) (*walmartclient.OrderLedger, error) {
c.calls++
if c.calls == 1 {
return nil, &url.Error{Op: "Get", URL: "https://www.walmart.com/order-ledger", Err: context.DeadlineExceeded}
}
return c.ledger, nil
}

func TestOrder_GetRefundItems_UsesClientReturnIDMetadata(t *testing.T) {
order := &Order{walmartOrder: &walmartclient.Order{
ID: "REFUND-ITEMS",
Expand All @@ -35,6 +50,26 @@ func TestOrder_GetRefundItems_UsesClientReturnIDMetadata(t *testing.T) {

// TestOrder_GetFinalCharges tests retrieving final charges from order ledger
func TestOrder_GetFinalCharges(t *testing.T) {
t.Run("retries a transient ledger timeout", func(t *testing.T) {
client := &flakyLedgerClient{ledger: &walmartclient.OrderLedger{
OrderID: "RETRY-LEDGER",
PaymentMethods: []walmartclient.PaymentMethodCharges{{
PaymentType: "CREDITCARD",
FinalCharges: []float64{6.06, 64.22},
}},
}}
order := &Order{
walmartOrder: &walmartclient.Order{ID: "RETRY-LEDGER"},
client: client,
}

charges, err := order.GetFinalCharges()

require.NoError(t, err)
assert.Equal(t, []float64{6.06, 64.22}, charges)
assert.Equal(t, 2, client.calls)
})

t.Run("single delivery order", func(t *testing.T) {
order := &Order{
walmartOrder: &walmartclient.Order{ID: "TEST123"},
Expand Down
48 changes: 40 additions & 8 deletions internal/application/sync/consolidator.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@ package sync
import (
"context"
"encoding/json"
"errors"
"fmt"
"log/slog"
"math"
"net"
"strings"
"time"

"github.com/eshaffer321/monarch-go/v2/pkg/monarch"
"github.com/eshaffer321/itemize/internal/adapters/providers"
"github.com/eshaffer321/itemize/internal/infrastructure/storage"
"github.com/eshaffer321/monarch-go/v2/pkg/monarch"
)

// Consolidator handles transaction consolidation for multi-delivery orders
Expand Down Expand Up @@ -159,6 +161,12 @@ func (c *Consolidator) updatePrimaryTransaction(
) (*monarch.Transaction, error) {
// Build consolidation note
note := c.buildConsolidationNote(allTransactions)
if math.Abs(math.Abs(primary.Amount)-math.Abs(order.GetTotal())) <= 0.01 &&
strings.HasPrefix(primary.Notes, "Multi-delivery order (") {
// A prior update may have reached Monarch even if its HTTP response timed
// out. Preserve the original charge audit note while deleting leftovers.
note = primary.Notes
}

// Calculate new amount (match sign of original transaction)
newAmount := order.GetTotal()
Expand Down Expand Up @@ -193,15 +201,27 @@ func (c *Consolidator) updatePrimaryTransaction(
Notes: &note,
}

start := time.Now()
updated, err := c.client.Transactions.Update(ctx, primary.ID, params)
duration := time.Since(start).Milliseconds()
const maxAttempts = 2
var updated *monarch.Transaction
var err error
for attempt := 1; attempt <= maxAttempts; attempt++ {
start := time.Now()
updated, err = c.client.Transactions.Update(ctx, primary.ID, params)
duration := time.Since(start).Milliseconds()

// Log API call
c.logAPICall(order.GetID(), "Transactions.Update", params, updated, err, duration)
// Log every attempt so timeout recovery remains auditable.
c.logAPICall(order.GetID(), "Transactions.Update", params, updated, err, duration)

if err != nil {
return nil, fmt.Errorf("failed to update transaction %s: %w", primary.ID, err)
if err == nil {
break
}
if attempt == maxAttempts || !isRetryableMonarchError(ctx, err) {
return nil, fmt.Errorf("failed to update transaction %s: %w", primary.ID, err)
}
c.logger.Warn("Transient transaction update failed; retrying",
"transaction_id", primary.ID,
"attempt", attempt,
"error", err)
}

c.logger.Info("Updated primary transaction",
Expand All @@ -212,6 +232,18 @@ func (c *Consolidator) updatePrimaryTransaction(
return updated, nil
}

func isRetryableMonarchError(ctx context.Context, err error) bool {
if ctx.Err() != nil || errors.Is(err, context.Canceled) {
return false
}
if monarch.IsRetryable(err) {
return true
}

var networkErr net.Error
return errors.As(err, &networkErr) && networkErr.Timeout()
}

// deleteExtraTransactions removes the extra transactions after consolidation
// Returns list of transaction IDs that failed to delete
func (c *Consolidator) deleteExtraTransactions(
Expand Down
Loading