Skip to content

chore(test): point Azure RI-exchange credential test comments at cli#932 - #1225

Merged
IAmJSD merged 1 commit into
LeanerCloud:mainfrom
IAmJSD:fix/issue-26
Oct 25, 2026
Merged

IAmJSD merged 1 commit into
LeanerCloud:mainfrom
IAmJSD:fix/issue-26

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

Root cause and fix

Merged LeanerCloud/cloud-commitments-cli#934 fixed LeanerCloud/cloud-commitments-cli#932 (Azure RI-exchange per-subscription credential resolution). Its branch was named fix/871-azure-ri-credentials, so two test comments for that work in internal/api/handler_ri_exchange_test.go cite issue #871. LeanerCloud/cloud-commitments-cli#871 is a different issue: the design question about whether RI Exchange should honor the Main Header global filter. The issue mentions lines ~811 and ~829. The file has grown since, and those comments are now at lines 1425 and 1443.

This PR changes only those two comments to issue LeanerCloud/cloud-commitments-cli#932. It keeps the file's issue #N wording but adds the repo name, because a bare #932 here would link to this repo's PR #932, which is unrelated.

Comment-only change. No code, test logic or behavior changes.

References left alone on purpose

I checked every 871 hit from git grep -n 871. These ones really do mean cli#871 (the RI Exchange global-filter scoping), so I left them unchanged:

  • internal/api/handler_ri_exchange.go:1451: listConvertibleRIs honoring the ?account_id= global account filter
  • internal/api/handler_ri_exchange_test.go:35 and :59: the AWS convertible-RI account-scope tests
  • frontend/src/...: the global-filter scoping comments. These are correct, and frontend is out of scope anyway.
  • The rest are unrelated: line numbers in docs/audits, a commit SHA in known_issues/, and hashes in go.work.sum and the Terraform lock files.

Verification

No regression test: nothing a test could check changes, since only comments change. Evidence is local, on macOS:

  • go build ./internal/api/: OK
  • go vet ./internal/api/: OK
  • gofmt -l internal/api/handler_ri_exchange_test.go: no output
  • git diff --check: clean
  • AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./internal/api/ -count=1 -v -run '^(<all 76 Test* funcs in handler_ri_exchange_test.go>)$': 76 PASS, 0 FAIL/SKIP. The AWS env vars only work around a broken local ~/.aws/config default profile.
  • AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test ./internal/mocks/ -count=1: ok
  • golangci-lint v2.10.1 run --allow-parallel-runners ./internal/api/: 0 issues
  • git-secrets 1.3.0 --scan --cached on the changed file, in a fresh git init temp repo with an isolated HOME, using scripts/setup-git-secrets.sh and .gitallowed: clean. scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.
  • gocyclo: no non-test Go changed, so nothing to check.

Open PRs that edit handler_ri_exchange_test.go (#1108, #1121, #1159, #1175, #1191) don't touch lines 1420-1450, so this PR shouldn't conflict with them.

Out of scope

  • The cli#934 branch name fix/871-azure-ri-credentials lives in the merged history of another repo and can't be changed.

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

Closes #26

🤖 Generated with Claude Code

The Azure credential-resolution tests in handler_ri_exchange_test.go cite
issue LeanerCloud#871, but that work is LeanerCloud/cloud-commitments-cli#932 (fixed by
cli#934, whose branch name carried the wrong number). cli#871 is the RI
Exchange global-filter design question, so the other LeanerCloud#871 references in the
file and in handler_ri_exchange.go are correct and stay as they are. Use the
full repo-qualified form so the reference cannot be read as this repo's LeanerCloud#932.

Comment-only change; no behavior change.

Closes LeanerCloud#26

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 1 billable file and costs up to $0.25.

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
Commits

Reviewing files that changed from the base of the PR and between 2f1dbca and 211b81b.

Files selected for processing: 1
  • internal/api/handler_ri_exchange_test.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@IAmJSD IAmJSD added impact/internal Team-internal only type/chore Maintenance / non-user-visible triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline effort/xs Trivial / one-liner labels Oct 20, 2026
@cristim

cristim commented Oct 25, 2026

Copy link
Copy Markdown
Member

Independent review 2 (claude-fleet, cold): APPROVE 211b81b

This is a comment-only change in internal/api/handler_ri_exchange_test.go (+4/-3), with no code or behaviour change.

Checked:

Findings: none.

Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged.

@cristim

cristim commented Oct 25, 2026

Copy link
Copy Markdown
Member

VERDICT: APPROVE 211b81b

Base: main (not stacked). git merge-tree --write-tree origin/main 211b81bb (origin/main 8f186a8): clean, no conflicts.

Fixes #26: yes. The only two credential-resolution comments in internal/api/handler_ri_exchange_test.go that cited #871 (the section header at :1425 and the TestListExchangeableAzureRIs_NoAzureAccountRegistered doc at :1443) now cite LeanerCloud/cloud-commitments-cli#932. That issue is titled "resolve Azure credentials via per-subscription resolver so Active Convertible RIs stop 500ing in Lambda" and was fixed by merged cli#934. Qualifying the reference with the repo name is right, because a bare #932 would link to an unrelated PR in this repo.

Findings: none.

Attacked:

  • Confirmed with gh: cli#871 is "Should RI Exchange page honor Main Header global filter?", cli#932 is the Azure credential issue, and cli#934 (branch fix/871-azure-ri-credentials) is MERGED.
  • git grep -n 871 on the head across internal/, cmd/ and frontend/src. The remaining hits (handler_ri_exchange.go:1451, handler_ri_exchange_test.go:35, and the frontend riexchange global-filter and Azure-listing comments) are about cli#871's scope (the global account filter), not credential resolution, so leaving them is correct. No other test comment in the credential block (lines 1425-1570) cites fix(purchase): isolate manual cleanup failures per plan #871.
  • gofmt on the changed file: clean. The diff is comment-only (4+/3-), so it changes no behavior and has no test impact. No mutation probe applies to a comment-only change.
  • The issue also mentions the cli#934 branch name. That is history in another repo and correctly left out of scope.

@IAmJSD

IAmJSD commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 211b81bbd558ea38b331613b4466e433e0b136a3

Blocking:

  • none

Non-blocking:

  • internal/api/handler_ri_exchange_test.go:35, :59 and internal/api/handler_ri_exchange.go:1451: these still say bare issue #871. They do mean cli#871 (the global-filter question), but by this PR's own reasoning a bare #871 in this repo points at platform#871, which is an unrelated merged PR ("fix(purchase): isolate manual cleanup failures per plan"). Qualifying them as LeanerCloud/cloud-commitments-cli#871 would finish the traceability cleanup. That's optional: chore(test): correct #871 -> #932 issue references in merged ri-exchange Azure cred tests #26 only asks about the two Azure credential comments.

Attacked:

  • Checked the cross-references with gh. cli#932 is "resolve Azure credentials via per-subscription resolver ... Active Convertible RIs stop 500ing in Lambda". cli#934 (branch fix/871-azure-ri-credentials) says Closes #932. cli#871 is the closed RI Exchange global-filter design question. So the two retargeted comments now point at the right issue, and the three #871 references left alone really are about global-filter scoping.
  • Comment-only diff (4+/3-), no code tokens touched. Ran go build ./internal/api/, go vet ./internal/api/ with and without -tags integration, gofmt -l (clean) and git diff --check (clean). No fail-before is applicable.

Body-stale: none

Reviewer: rB3. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.

@IAmJSD

IAmJSD commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification at 211b81bbd558ea38b331613b4466e433e0b136a3 merged onto main (5aa3df5b), together with #1155, #1219, #1221, #1223, #1224, #1225, #1226, #1227, #1256, #1262, #1266, #1270, #1272, #1275, #1283, #1286, #1293, #1297, #1299, #1300, #1301, #1304, #1307, #1311, #1313, #1325 in that order (combined tree f3a3331f, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 2 approving independent review(s) on it. Nothing was run against any cloud.

@IAmJSD
IAmJSD marked this pull request as ready for review October 25, 2026 20:24
@IAmJSD
IAmJSD merged commit df9c455 into LeanerCloud:main Oct 25, 2026
25 checks passed
@IAmJSD

IAmJSD commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 2 approving independent review(s) on 211b81bb (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly 211b81bbd558ea38b331613b4466e433e0b136a3.

This was referenced Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/eventually No deadline

Projects

None yet

2 participants