fix(cli): honor --idempotency-window in the duplicate-purchase check

README.md

@@ -22,7 +22,7 @@ The CLI depends on the published shared Go modules in [cloud-commitments-go](htt
 4. **RDS extended-support filtering** - by default, recommendations for instances running an engine version in AWS Extended Support are excluded, since the surcharge can erase RI savings; pass `--include-extended-support` to include them.
 5. **Audit log written per recommendation** - the audit log path is checked for writability before any cloud API call. Each recommendation then gets its own audit record: for a dry run, written as soon as its (local, no-API-call) result is generated; for a real purchase, written after that purchase call returns.
 6. **Permanent CSV exports** of every dry run and every purchase.
-7. **Duplicate-purchase dedup, fails closed** - every path (`--services` and `--input-csv`) subtracts commitments purchased in the last 24 hours before sizing a recommendation. `--idempotency-window` doesn't change that fixed 24h lookback yet ([#1262](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1262)). If the existing-commitments API call itself fails, a dry run continues with a warning (nothing is bought); a `--purchase` run refuses that (service, region) with a "Refusing to purchase" line and buys nothing there.
+7. **Duplicate-purchase dedup, fails closed** - every path (`--services` and `--input-csv`) subtracts commitments purchased within `--idempotency-window` (default `24h`, whole hours only; an invalid value is rejected at startup) before sizing a recommendation. If the existing-commitments API call itself fails, a dry run continues with a warning (nothing is bought); a `--purchase` run refuses that (service, region) with a "Refusing to purchase" line and buys nothing there.
 
 Full internals: [Purchase Safety](docs/cli/purchase-safety.md).
 

cmd/main.go

@@ -62,6 +62,7 @@ type Config struct {
 	TermYears              int
 	CoverageLookbackDays   int
 	MinCount               int
+	IdempotencyWindowHours int
 	MinSavingsPct          float64
 	OverrideCount          int32
 	MaxInstances           int32

cmd/multi_service_helpers.go

@@ -203,7 +203,7 @@ func populateAccountNames(ctx context.Context, recs []common.Recommendation, acc
 
 // adjustRecsForDuplicates checks for existing RIs and adjusts recommendations to avoid duplicates.
 func adjustRecsForDuplicates(ctx context.Context, recs []common.Recommendation, serviceClient provider.ServiceClient) ([]common.Recommendation, error) {
-	duplicateChecker := NewDuplicateChecker(0)
+	duplicateChecker := NewDuplicateChecker(toolCfg.IdempotencyWindowHours)
 	adjustedRecs, _, err := duplicateChecker.AdjustRecommendationsForExisting(ctx, recs, serviceClient)
 	if err != nil {
 		return recs, err // Return original recommendations with error
@@ -598,7 +598,7 @@ func checkDuplicates(
 	drops *common.DropSummary,
 ) []common.Recommendation {
 	// Check for duplicate RIs to avoid double purchasing
-	duplicateChecker := NewDuplicateChecker(0)
+	duplicateChecker := NewDuplicateChecker(toolCfg.IdempotencyWindowHours)
 	adjustedRecs, dedupedOut, err := duplicateChecker.AdjustRecommendationsForExistingRIs(ctx, filteredRecs, serviceClient)
 	if err != nil {
 		if !isDryRun {

cmd/validators.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 	"github.com/spf13/cobra"
@@ -37,6 +38,26 @@ func validateFlags(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	if err := validateIdempotencyWindow(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// validateIdempotencyWindow parses --idempotency-window into the whole hours
+// the duplicate check's lookback is measured in. Anything it cannot represent
+// exactly is rejected rather than rounded, since a shorter window than asked
+// for lets a duplicate purchase through.
+func validateIdempotencyWindow() error {
+	window, err := time.ParseDuration(toolCfg.IdempotencyWindow)
+	if err != nil {
+		return fmt.Errorf("invalid idempotency-window %q: %w", toolCfg.IdempotencyWindow, err)
+	}
+	if window <= 0 || window%time.Hour != 0 {
+		return fmt.Errorf("invalid idempotency-window %q: must be a positive whole number of hours (e.g. 24h, 72h)", toolCfg.IdempotencyWindow)
+	}
+	toolCfg.IdempotencyWindowHours = int(window / time.Hour)
 	return nil
 }
 

cmd/validators_test.go

@@ -1,10 +1,12 @@
 package main
 
 import (
+	"context"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 	"github.com/spf13/cobra"
@@ -686,3 +688,76 @@ func TestValidateCSVModeFilterFlags(t *testing.T) {
 		})
 	}
 }
+
+// TestValidateIdempotencyWindow covers #1262: the flag used to be accepted
+// unparsed, so "banana" or "90m" ran silently with a hardcoded 24h window.
+func TestValidateIdempotencyWindow(t *testing.T) {
+	tests := []struct {
+		window    string
+		wantHours int
+		wantErr   bool
+	}{
+		{window: "24h", wantHours: 24},
+		{window: "72h", wantHours: 72},
+		{window: "1h", wantHours: 1},
+		{window: "", wantErr: true},
+		{window: "banana", wantErr: true},
+		{window: "0h", wantErr: true},
+		{window: "-24h", wantErr: true},
+		{window: "90m", wantErr: true},
+		{window: "1h30m", wantErr: true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.window, func(t *testing.T) {
+			origCfg := toolCfg
+			defer func() { toolCfg = origCfg }()
+			toolCfg.IdempotencyWindow = tt.window
+			err := validateIdempotencyWindow()
+			if tt.wantErr {
+				if err == nil || !strings.Contains(err.Error(), "invalid idempotency-window") {
+					t.Fatalf("validateIdempotencyWindow(%q) error = %v, want invalid idempotency-window", tt.window, err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("validateIdempotencyWindow(%q) unexpected error = %v", tt.window, err)
+			}
+			if toolCfg.IdempotencyWindowHours != tt.wantHours {
+				t.Errorf("IdempotencyWindowHours = %d, want %d", toolCfg.IdempotencyWindowHours, tt.wantHours)
+			}
+		})
+	}
+}
+
+// TestCheckDuplicates_HonorsIdempotencyWindow reproduces the #1262 scenario:
+// a re-run 30h after a purchase with --idempotency-window 72h must subtract
+// that purchase. With the window ignored the lookback stayed at 24h and the
+// same 5 RIs were bought again.
+func TestCheckDuplicates_HonorsIdempotencyWindow(t *testing.T) {
+	origCfg := toolCfg
+	defer func() { toolCfg = origCfg }()
+	toolCfg.IdempotencyWindow = "72h"
+	if err := validateIdempotencyWindow(); err != nil {
+		t.Fatalf("validateIdempotencyWindow: %v", err)
+	}
+
+	ctx := context.Background()
+	recs := []common.Recommendation{{
+		ResourceType: "db.t3.small", Region: "us-east-1", Count: 5,
+		Details: &common.DatabaseDetails{Engine: "mysql"},
+	}}
+	existing := []common.Commitment{{
+		ResourceType: "db.t3.small", Region: "us-east-1", Engine: "mysql",
+		Count: 5, State: "active", StartDate: time.Now().Add(-30 * time.Hour),
+	}}
+	mockClient := &MockServiceClient{}
+	mockClient.On("GetExistingCommitments", ctx).Return(existing, nil)
+	t.Cleanup(func() { mockClient.AssertExpectations(t) })
+
+	if got := checkDuplicates(ctx, recs, mockClient, false /* isDryRun */, nil); CalculateTotalInstances(got) != 0 {
+		t.Errorf("checkDuplicates kept %d instance(s); the 30h-old purchase is inside the 72h window and must be subtracted", CalculateTotalInstances(got))
+	}
+	if got, err := adjustRecsForDuplicates(ctx, recs, mockClient); err != nil || CalculateTotalInstances(got) != 0 {
+		t.Errorf("adjustRecsForDuplicates (CSV path) kept %d instance(s), err=%v; want 0", CalculateTotalInstances(got), err)
+	}
+}

docs/cli/README.md

@@ -50,7 +50,7 @@ All flags belong to the root command unless noted otherwise.
 | `--purchase` | | `false` | Execute real purchases. This is the only purchase control: a bare run is always a dry run, and `--purchase` alone executes real purchases (identically in cloud-fetch and `--input-csv` modes). Still gated by the `--yes` / interactive confirmation prompt. See [purchase-safety.md](purchase-safety.md). |
 | `--yes` | | `false` | Skip the interactive confirmation prompt. Use with caution in automation. |
 | `--audit-log` | | `./cudly-audit.jsonl` | Path to the JSONL audit log file. Written for every recommendation (dry-run and real). See [purchase-safety.md](purchase-safety.md). |
-| `--idempotency-window` | | `24h` | Lookback window for duplicate purchase detection. Accepted as a Go duration string (not validated by the CLI; currently has no effect on CLI runs). See [purchase-safety.md](purchase-safety.md). |
+| `--idempotency-window` | | `24h` | Lookback window for duplicate purchase detection. A Go duration string that must be a positive whole number of hours (e.g. `24h`, `72h`); anything else is rejected at startup. See [purchase-safety.md](purchase-safety.md). |
 
 ### Recommendation quality filters
 

docs/cli/purchase-safety.md

@@ -84,16 +84,16 @@ The default path (`./cudly-audit.jsonl`) writes to the current working directory
 --idempotency-window   string   default: 24h
 ```
 
-A duplicate check runs before every purchase, on both the `--services` and `--input-csv` paths: it fetches existing commitments and subtracts anything purchased in the last 24 hours from each recommendation's count, so a retried run doesn't buy the same capacity twice.
+A duplicate check runs before every purchase, on both the `--services` and `--input-csv` paths: it fetches existing commitments and subtracts anything purchased within the window from each recommendation's count, so a retried run doesn't buy the same capacity twice.
 
-That 24-hour lookback is fixed. This flag is accepted as a Go duration string (e.g. `24h`, `48h`, `1h30m`) and stored, but its value is never read by the check - passing `--idempotency-window 72h` (or any other value) has no effect on which recommendations are purchased ([#1262](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1262) tracks wiring it in).
+The window is a Go duration string that must be a positive whole number of hours (e.g. `24h`, `48h`, `72h`). A value that doesn't parse, is zero or negative, or isn't whole hours (e.g. `90m`, `1h30m`) is rejected at startup, before any API call, rather than rounded or replaced with the default.
 
 If the existing-commitments lookup itself fails (a transient API error), the two modes diverge: a dry run continues with a warning printed to the log (nothing is bought, so reporting fidelity wins), while a `--purchase` run refuses that (service, region) - it prints a "Refusing to purchase" line and buys nothing there, rather than falling back to the un-deduplicated counts ([#1941](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1941)).
 
 The audit status value `skipped_covered` (idempotency hit) is defined in the audit record schema for use by the server-side scheduler path and is not emitted by this CLI's dedup check.
 
 ```bash
-# The dedup check always runs with a fixed 24h lookback; this flag's value is not applied:
+# Subtract anything purchased in the last 72 hours:
 cudly --services rds --idempotency-window 72h
 ```
 
@@ -139,5 +139,5 @@ Before any real purchase run:
 3. If using `--target-coverage`, verify `--rebuy-window-days` is set appropriately for your RI renewal cadence.
 4. Narrow the scope with `--include-regions`, `--include-accounts`, or `--min-savings-pct` before buying across all services.
 5. Consider `--max-instances` as a final safety cap for a first run.
-6. Note that `--idempotency-window`'s value is not applied - dedup always uses a fixed 24h lookback ([#1262](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1262)) - and that a failed existing-commitments lookup makes a dry run proceed un-deduplicated with a warning, while a `--purchase` run refuses to purchase for that (service, region) instead ([#1941](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1941)); watch the log for either signal and check the audit log afterward.
+6. Set `--idempotency-window` to cover the time since the earlier run (e.g. `72h` for a re-run two days later), and note that a failed existing-commitments lookup makes a dry run proceed un-deduplicated with a warning, while a `--purchase` run refuses to purchase for that (service, region) instead ([#1941](https://github.com/LeanerCloud/cloud-commitments-cli/issues/1941)); watch the log for either signal and check the audit log afterward.
 7. If an AI agent or other automation drives `cudly`, never pass `--yes` to it directly - have the agent hand off the dry-run recommendation to a human, who runs `--purchase` themselves. See [Automation and AI agents](#automation-and-ai-agents).