fix(cmd): skip extended-support lifecycle entries with no start date

[IAmJSD]

Root cause

isInExtendedSupport (cmd/multi_service_engine_versions.go) gates an open-source-rds-extended-support lifecycle entry with now.Before(lifecycle.LifecycleSupportStartDate). When the start date is zero, that check is always false, so the start gate is skipped. With a zero end date, which is treated as open-ended since #1182, an entry with both dates missing came back as (extended=true, known=true). The instance was then dropped from the RDS recommendation count without any log line.

Fix

• An extended-support entry with LifecycleSupportStartDate.IsZero() is skipped (continue) instead of being accepted. A warning that names the engine and major version is logged: ⚠️ Extended-support lifecycle for mysql 5.7 has no start date: ignoring the entry, extended-support window unknown.
• If no other well-formed extended entry proves the window is active, the function returns known=false. This uses the existing isInExtendedSupport silently returns false on lifecycle lookup miss (money path) #1313 contract: shouldExcludeForExtendedSupport keeps the instance in the count (the conservative choice) and logs its existing No lifecycle data for ... warning, deduplicated per engine and version.
• A well-formed active extended entry next to a malformed one still returns (true, true).
• Standard-support entries are unaffected.

gocyclo for isInExtendedSupport is 9 (the cap is 10).

Closes #1315

Verification

All of this is unit-level evidence that uses in-memory MajorEngineVersionInfo fixtures. Nothing calls AWS. A real DescribeDBMajorEngineVersions response with a missing start date has not been seen in practice, so there is no real-cloud coverage for this path.

New tests in cmd/multi_service_engine_versions_test.go:

• TestIsInExtendedSupport cases:
  • extended entry, start zero, end zero → (false, false)
  • extended entry, start zero, end in the future → (false, false)
  • zero-start entry next to a valid active entry → (true, true)
  • zero-start standard-support entry → (false, true)
• TestIsInExtendedSupport_ZeroStartDateWarns: goes through adjustRecommendationForExcludedVersions. It asserts that the count stays at 2, that the log contains mysql 5.7 has no start date, and that the caller's No lifecycle data for mysql 5.7.44 warning appears.

Fail-before (fix stashed, tests kept):

$ AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./cmd -run 'TestIsInExtendedSupport'
--- FAIL: TestIsInExtendedSupport            (4x expected: false, actual: true)
--- FAIL: TestIsInExtendedSupport_ZeroStartDateWarns
        expected: 2, actual: 1   ("🚫 Found extended support instance ... 2 instances → 1 instances")
FAIL

Pass-after:

$ AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./cmd -run 'TestIsInExtendedSupport' -v
--- PASS: TestIsInExtendedSupport (all 13 cases)
--- PASS: TestIsInExtendedSupport_ZeroStartDateWarns
ok

Full checks, all passing:

go build -o <tmp>/cudly ./cmd
go vet ./...
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test -short -race ./...   # ok cmd
golangci-lint run --allow-parallel-runners ./...   # v2.10.1: 0 issues.
gocyclo -over 10 cmd/multi_service_engine_versions.go   # silent
gofmt -l cmd; make tidy-check; git diff --check   # clean

Tests were run with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null because the default AWS profile on the dev machine is invalid. This is not related to this change.

Out of scope

• The issue lists a metric for missing-date payloads as optional. This PR only adds the log line, because the CLI has no metrics plumbing on this path.
• The new warning is logged once per isInExtendedSupport call, which means once per matching instance. It is not deduplicated. The caller's No lifecycle data warning is still deduplicated per engine:version.
• cmd/multi_service_engine_versions.go was already longer than the 500-line guideline before this PR (571 lines). This PR adds 13 lines.

Maintainer: please mirror the issue's triage labels onto this PR: triaged, priority/p3, severity/low, urgency/eventually, impact/few, effort/xs, type/bug.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 2 billable files and costs up to $0.50.}

Or wait 57 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 01a28467-052d-4f1a-ab66-54595883dc41

📥 Commits

Reviewing files that changed from the base of the PR and between 028551d and 0fc9166.

📒 Files selected for processing: 2

• cmd/multi_service_engine_versions.go
• cmd/multi_service_engine_versions_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Merge-gate review verdict: CLEAN code at ca81b5f; rebase needed

Independent adversarial review by a non-author agent (Sonnet high, cold brief, full diff 817ab4bc...ca81b5f2): no blocking code findings.

@IAmJSD, please rebase onto current main. git merge-tree shows a content conflict in cmd/multi_service_engine_versions_test.go: #2207 (merged as 1836029) deleted TestQueryMajorEngineVersions_ErrorHandling, and this PR inserts its new test right before it. Production code does not conflict. After the rebase I will re-check the range-diff against ca81b5f. If the only change is the conflict resolution, this verdict carries over.

Checked:

• Decision semantics. If the only extended-support entry for a version has a zero start date, isInExtendedSupport returns (extended=false, known=false). Its single consumer, shouldExcludeForExtendedSupport (cmd/multi_service_engine_versions.go:562-574), logs "No lifecycle data..." and keeps the instance counted, so the version is treated as unknown. That matches isInExtendedSupport silently accepts zero LifecycleSupportStartDate #1315 and the isInExtendedSupport silently returns false on lifecycle lookup miss (money path) #1313 policy. With a valid active entry alongside, the result is (true, true). Entries with valid dates behave as before; the start and end comparisons and zero-end-means-open-ended are unchanged.
• SDK semantics. LifecycleSupportStartDate and LifecycleSupportEndDate are required *time.Time (aws-sdk-go-v2 rds v1.97.3 types/types.go:5113, :5143). The fetch code (:322-327) maps nil to a zero time.Time, so IsZero() is the right check and there is no nil dereference.

Non-blocking note: any extended entry missing a start date makes the result unknown, even if another entry for the same version has already ended. This is consistent with the issue.

Local verification on macOS at ca81b5f (fixture/mock-based, no AWS):

• go test ./... -count=1 (go1.26.9, AWS config and credentials set to /dev/null): ok cmd 70.2s. go build ./cmd OK; go vet ./... clean.
• Fail-before: with the 817ab4b production file and this PR's tests, 3 tests fail (both zero-start table cases and TestIsInExtendedSupport_ZeroStartDateWarns). They pass at head.
• Deletion probe: replacing the IsZero() check with false fails the same tests. The probe was reverted.
• Not tested: a real AWS payload (the anomalous nil-date response is not observable in practice).

Merge blockers:

1. Rebase onto main (author).
2. Approve the fork CI runs for the rebased head (maintainer).
3. Mark the PR ready (author).

🤖 Generated with Claude Code

[IAmJSD]

Thanks @cristim. The branch is now up to date with main at 0fc9166.

I merged origin/main instead of rebasing, to avoid force-pushing over the reviewed commits. ca81b5f2 is still in the history, so git diff ca81b5f2 0fc9166c -- cmd/multi_service_engine_versions.go shows only main's changes, and the production file is unchanged by the resolution.

Conflict resolution in cmd/multi_service_engine_versions_test.go:

• Kept this PR's TestIsInExtendedSupport_ZeroStartDateWarns unchanged.
• Dropped TestQueryMajorEngineVersions_ErrorHandling, as test(cli): make engine-version error-handling tests hermetic #2207 deleted it.
• Kept main's TestQueryRunningInstanceEngineVersions_ProfileSelection.

Checked on macOS at 0fc9166, with AWS config and credentials set to /dev/null: go build OK, go vet ./... clean, go test ./... -count=1 ok, golangci-lint v2.10.1 reports 0 issues, and gofmt is clean.

The fork CI run for the new head needs approval.

🤖 Generated with Claude Code

[cristim]

Verdict carry-over: CLEAN at 0fc9166 (merge of main into ca81b5f)

The new head, 0fc9166, merges main into ca81b5f. The PR's own changes are identical to what was reviewed: git diff 817ab4bc ca81b5f2 and git diff 028551dc 0fc9166c contain the same added and removed lines. The merge resolved the conflict in cmd/multi_service_engine_versions_test.go without adding any content of its own. git merge-tree against the current main (a2c4c7e) is clean.

I re-ran the checks on macOS at 0fc9166. go test ./... -count=1 (go1.26.9, AWS config and credentials set to /dev/null) passed with ok cmd 25.6s, and go vet ./... was clean. These are fixture/mock-based tests.

The CLEAN verdict from the earlier comment therefore stands for 0fc9166.

Remaining merge blockers:

• A maintainer needs to approve fork CI runs 38872782081 and 38872782101.
• The author needs to mark the PR ready.
• CodeRabbit then needs to run, or be waived for quota.

🤖 Generated with Claude Code