diff --git a/docs/bug-fixes.md b/docs/bug-fixes.md index 6fc788e..0ab091d 100644 --- a/docs/bug-fixes.md +++ b/docs/bug-fixes.md @@ -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:** diff --git a/go.mod b/go.mod index daa60e6..38509d6 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 88c4f56..521ff1a 100644 --- a/go.sum +++ b/go.sum @@ -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= @@ -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= diff --git a/internal/adapters/providers/walmart/order.go b/internal/adapters/providers/walmart/order.go index ffd9cbb..cfb4b06 100644 --- a/internal/adapters/providers/walmart/order.go +++ b/internal/adapters/providers/walmart/order.go @@ -2,8 +2,10 @@ package walmart import ( "context" + "errors" "fmt" "log/slog" + "net" "time" "github.com/eshaffer321/itemize/internal/adapters/providers" @@ -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 diff --git a/internal/adapters/providers/walmart/order_multi_delivery_test.go b/internal/adapters/providers/walmart/order_multi_delivery_test.go index 4b77e12..c6aac18 100644 --- a/internal/adapters/providers/walmart/order_multi_delivery_test.go +++ b/internal/adapters/providers/walmart/order_multi_delivery_test.go @@ -1,8 +1,10 @@ package walmart import ( + "context" "io" "log/slog" + "net/url" "testing" walmartclient "github.com/eshaffer321/walmart-client-go/v2" @@ -10,6 +12,19 @@ import ( "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", @@ -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"}, diff --git a/internal/application/sync/consolidator.go b/internal/application/sync/consolidator.go index ede4ff7..19e005e 100644 --- a/internal/application/sync/consolidator.go +++ b/internal/application/sync/consolidator.go @@ -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 @@ -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() @@ -193,15 +201,27 @@ func (c *Consolidator) updatePrimaryTransaction( Notes: ¬e, } - 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", @@ -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( diff --git a/internal/application/sync/consolidator_test.go b/internal/application/sync/consolidator_test.go index 6efb010..e5d00f7 100644 --- a/internal/application/sync/consolidator_test.go +++ b/internal/application/sync/consolidator_test.go @@ -4,11 +4,12 @@ import ( "context" "fmt" "log/slog" + "net/url" "testing" "time" - "github.com/eshaffer321/monarch-go/v2/pkg/monarch" "github.com/eshaffer321/itemize/internal/adapters/providers" + "github.com/eshaffer321/monarch-go/v2/pkg/monarch" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -20,22 +21,23 @@ type mockOrder struct { total float64 } -func (m *mockOrder) GetID() string { return m.id } -func (m *mockOrder) GetDate() time.Time { return m.date } -func (m *mockOrder) GetTotal() float64 { return m.total } -func (m *mockOrder) GetSubtotal() float64 { return 0 } -func (m *mockOrder) GetTax() float64 { return 0 } -func (m *mockOrder) GetTip() float64 { return 0 } -func (m *mockOrder) GetFees() float64 { return 0 } -func (m *mockOrder) GetItems() []providers.OrderItem { return nil } -func (m *mockOrder) GetProviderName() string { return "test" } -func (m *mockOrder) GetRawData() interface{} { return nil } +func (m *mockOrder) GetID() string { return m.id } +func (m *mockOrder) GetDate() time.Time { return m.date } +func (m *mockOrder) GetTotal() float64 { return m.total } +func (m *mockOrder) GetSubtotal() float64 { return 0 } +func (m *mockOrder) GetTax() float64 { return 0 } +func (m *mockOrder) GetTip() float64 { return 0 } +func (m *mockOrder) GetFees() float64 { return 0 } +func (m *mockOrder) GetItems() []providers.OrderItem { return nil } +func (m *mockOrder) GetProviderName() string { return "test" } +func (m *mockOrder) GetRawData() interface{} { return nil } // mockMonarchClient simulates the Monarch client for testing type mockMonarchClient struct { updateCalled int deleteCalled int updateError error + updateErrors []error deleteError error deleteFailAfter int // Fail after N deletes (for partial failure testing) } @@ -98,6 +100,9 @@ func (m *mockMonarchClient) Create(ctx context.Context, params *monarch.CreateTr // Update updates a transaction func (m *mockMonarchClient) Update(ctx context.Context, id string, params *monarch.UpdateTransactionParams) (*monarch.Transaction, error) { m.updateCalled++ + if len(m.updateErrors) >= m.updateCalled && m.updateErrors[m.updateCalled-1] != nil { + return nil, m.updateErrors[m.updateCalled-1] + } if m.updateError != nil { return nil, m.updateError } @@ -262,6 +267,25 @@ func TestConsolidator_ConsolidateTransactions(t *testing.T) { assert.Equal(t, 0, mockClient.deleteCalled, "Should not delete in dry-run") }) + t.Run("resumed consolidation preserves the original charge note", func(t *testing.T) { + mockClient := &mockMonarchClient{} + client := &monarch.Client{Transactions: mockClient} + consolidator := NewConsolidator(client, logger, nil, 0) + originalNote := "Multi-delivery order (2 charges: $6.06, $64.22)" + + order := &mockOrder{id: "RESUME", total: 70.28} + transactions := []*monarch.Transaction{ + {ID: "primary", Amount: -70.28, Notes: originalNote}, + {ID: "extra", Amount: -64.22}, + } + + result, err := consolidator.ConsolidateTransactions(ctx, transactions, order, true) + + require.NoError(t, err) + assert.Equal(t, originalNote, result.ConsolidatedTransaction.Notes) + assert.NotContains(t, result.ConsolidatedTransaction.Notes, "$70.28") + }) + t.Run("handles positive transaction amounts (returns)", func(t *testing.T) { mockClient := &mockMonarchClient{} client := &monarch.Client{ @@ -349,6 +373,30 @@ func TestConsolidator_ConsolidateTransactions(t *testing.T) { assert.Contains(t, err.Error(), "failed to update primary transaction") }) + t.Run("retries a transient primary update timeout", func(t *testing.T) { + mockClient := &mockMonarchClient{ + updateErrors: []error{ + &url.Error{Op: "Post", URL: "https://api.monarch.com/graphql", Err: context.DeadlineExceeded}, + nil, + }, + } + client := &monarch.Client{Transactions: mockClient} + consolidator := NewConsolidator(client, logger, nil, 0) + + order := &mockOrder{id: "RETRY-UPDATE", total: 70.28} + transactions := []*monarch.Transaction{ + {ID: "txn1", Amount: -6.06}, + {ID: "txn2", Amount: -64.22}, + } + + result, err := consolidator.ConsolidateTransactions(ctx, transactions, order, false) + + require.NoError(t, err) + assert.Equal(t, 2, mockClient.updateCalled) + assert.Equal(t, 1, mockClient.deleteCalled) + assert.Equal(t, -70.28, result.ConsolidatedTransaction.Amount) + }) + t.Run("partial failure - some deletions fail", func(t *testing.T) { mockClient := &mockMonarchClient{ deleteFailAfter: 1, // First delete succeeds, second fails diff --git a/internal/application/sync/handlers/walmart.go b/internal/application/sync/handlers/walmart.go index ffa5b97..fc8fb3d 100644 --- a/internal/application/sync/handlers/walmart.go +++ b/internal/application/sync/handlers/walmart.go @@ -342,6 +342,7 @@ func (h *WalmartHandler) processMultiDeliveryOrder( catCategories, monarchCategories, charges, + multiResult.Matches, dryRun, ) if aggregateErr != nil { @@ -399,6 +400,7 @@ func (h *WalmartHandler) processMultiDeliveryAggregateFallback( catCategories []categorizer.Category, monarchCategories []*monarch.TransactionCategory, charges []float64, + partialMatches []*matcher.MatchResult, dryRun bool, ) (*ProcessResult, error) { ledgerTotal := sumCharges(charges) @@ -425,6 +427,25 @@ func (h *WalmartHandler) processMultiDeliveryAggregateFallback( } if len(matchedTxns) == 1 { + recoveryTxns := interruptedConsolidationTransactions(matchedTxns[0], partialMatches, charges, ledgerTotal) + if len(recoveryTxns) > 1 { + if h.consolidator == nil { + return nil, fmt.Errorf("interrupted consolidation found %d undeleted transactions but consolidator is not configured", len(recoveryTxns)-1) + } + for _, txn := range recoveryTxns[1:] { + usedTxnIDs[txn.ID] = true + } + h.logInfo("Resuming interrupted multi-delivery consolidation", + "order_id", order.GetID(), + "consolidated_id", matchedTxns[0].ID, + "undeleted_count", len(recoveryTxns)-1) + consolidationResult, err := h.consolidator.ConsolidateTransactions(ctx, recoveryTxns, matchOrder, dryRun) + if err != nil { + return nil, fmt.Errorf("interrupted consolidation recovery error: %w", err) + } + return h.categorizeAndApplySplits(ctx, order, consolidationResult.ConsolidatedTransaction, catCategories, monarchCategories, dryRun) + } + h.logInfo("Matched multi-delivery order by aggregate ledger total", "order_id", order.GetID(), "ledger_charge_count", len(charges), @@ -451,6 +472,39 @@ func (h *WalmartHandler) processMultiDeliveryAggregateFallback( return h.categorizeAndApplySplits(ctx, order, consolidationResult.ConsolidatedTransaction, catCategories, monarchCategories, dryRun) } +func interruptedConsolidationTransactions( + primary *monarch.Transaction, + partialMatches []*matcher.MatchResult, + charges []float64, + ledgerTotal float64, +) []*monarch.Transaction { + if primary == nil || math.Abs(math.Abs(primary.Amount)-math.Abs(ledgerTotal)) > 0.01 { + return nil + } + if primary.Notes != multiDeliveryConsolidationNote(charges) { + return nil + } + + transactions := []*monarch.Transaction{primary} + seen := map[string]bool{primary.ID: true} + for _, match := range partialMatches { + if match == nil || match.Transaction == nil || seen[match.Transaction.ID] { + continue + } + seen[match.Transaction.ID] = true + transactions = append(transactions, match.Transaction) + } + return transactions +} + +func multiDeliveryConsolidationNote(charges []float64) string { + formatted := make([]string, len(charges)) + for i, charge := range charges { + formatted[i] = fmt.Sprintf("$%.2f", math.Abs(charge)) + } + return fmt.Sprintf("Multi-delivery order (%d charges: %s)", len(charges), strings.Join(formatted, ", ")) +} + func countFoundMatches(matches []*matcher.MatchResult) int { foundCount := 0 for _, match := range matches { diff --git a/internal/application/sync/handlers/walmart_test.go b/internal/application/sync/handlers/walmart_test.go index 662fe39..9d2b33f 100644 --- a/internal/application/sync/handlers/walmart_test.go +++ b/internal/application/sync/handlers/walmart_test.go @@ -609,6 +609,47 @@ func TestWalmartHandler_ProcessOrder_MultiDelivery_FallsBackToAggregateTransacti assert.True(t, usedTxnIDs["txn-aggregate"]) } +func TestWalmartHandler_ProcessOrder_MultiDelivery_ResumesInterruptedConsolidation(t *testing.T) { + splitter := &walmartTestSplitter{categoryID: "groceries", notes: "Groceries"} + monarchClient := &walmartTestMonarch{} + orderDate := time.Now() + consolidated := &monarch.Transaction{ + ID: "partially-consolidated", + Amount: -70.28, + Notes: "Multi-delivery order (2 charges: $6.06, $64.22)", + Date: walmartToMonarchDate(orderDate), + } + consolidator := &walmartTestConsolidator{ + result: &ConsolidationResult{ConsolidatedTransaction: consolidated}, + } + handler := createTestWalmartHandler(t, splitter, consolidator, monarchClient) + + order := &walmartTestOrder{ + id: "INTERRUPTED-CONSOLIDATION", + date: orderDate, + total: 70.28, + subtotal: 65.00, + tax: 5.28, + items: []providers.OrderItem{&walmartTestItem{name: "Item", price: 65.00, quantity: 1}}, + charges: []float64{6.06, 64.22}, + } + txns := []*monarch.Transaction{ + consolidated, + {ID: "undeleted-extra", Amount: -64.22, Date: walmartToMonarchDate(orderDate)}, + } + usedTxnIDs := make(map[string]bool) + + result, err := handler.ProcessOrder(context.Background(), order, txns, usedTxnIDs, nil, nil, true) + + require.NoError(t, err) + assert.True(t, result.Processed) + require.Len(t, consolidator.receivedTransactions, 2) + assert.Equal(t, "partially-consolidated", consolidator.receivedTransactions[0].ID) + assert.Equal(t, "undeleted-extra", consolidator.receivedTransactions[1].ID) + assert.True(t, usedTxnIDs["partially-consolidated"]) + assert.True(t, usedTxnIDs["undeleted-extra"]) +} + func TestWalmartHandler_ProcessOrder_MultiDelivery_FallsBackToAggregateSubset(t *testing.T) { splitter := &walmartTestSplitter{categoryID: "groceries", notes: "Groceries"} monarchClient := &walmartTestMonarch{} diff --git a/internal/cli/output.go b/internal/cli/output.go index 45bd454..82dd0a8 100644 --- a/internal/cli/output.go +++ b/internal/cli/output.go @@ -70,7 +70,15 @@ func PrintSyncSummary(result *sync.Result, store storage.Repository, dryRun bool } } - if !dryRun && (result.ProcessedCount > 0 || result.RefundProcessedCount > 0) { - fmt.Println("\nSync completed successfully.") + if !dryRun { + if result.ErrorCount > 0 { + suffix := "errors" + if result.ErrorCount == 1 { + suffix = "error" + } + fmt.Printf("\nSync completed with %d %s.\n", result.ErrorCount, suffix) + } else if result.ProcessedCount > 0 || result.RefundProcessedCount > 0 { + fmt.Println("\nSync completed successfully.") + } } } diff --git a/internal/cli/output_test.go b/internal/cli/output_test.go index 0f5ea5c..bd8523d 100644 --- a/internal/cli/output_test.go +++ b/internal/cli/output_test.go @@ -27,3 +27,21 @@ func TestPrintSyncSummaryReportsAmazonRefundOutcome(t *testing.T) { assert.Contains(t, string(output), "Amazon refunds: Categorized=3 Left untouched=1") assert.Contains(t, string(output), "Sync completed successfully.") } + +func TestPrintSyncSummaryDoesNotReportSuccessWhenOrdersFailed(t *testing.T) { + previous := os.Stdout + reader, writer, err := os.Pipe() + require.NoError(t, err) + os.Stdout = writer + t.Cleanup(func() { os.Stdout = previous }) + + PrintSyncSummary(&sync.Result{ProcessedCount: 1, ErrorCount: 2}, nil, false) + require.NoError(t, writer.Close()) + os.Stdout = previous + output, err := io.ReadAll(reader) + require.NoError(t, err) + require.NoError(t, reader.Close()) + + assert.Contains(t, string(output), "Sync completed with 2 errors.") + assert.NotContains(t, string(output), "Sync completed successfully.") +}