Reorder config precedence: global flag beats environment variable

[peterSirotnak]

Fixes the precedence order for configuration value resolution so that an explicit --client-version CLI flag takes priority over an ambient CLIENT_VERSION environment variable. This ensures that a run passing --client-version=X installs X even when CLIENT_VERSION=Y is exported (e.g. by CI).

Changes:

• Reordered resolve_value() logic in lib/config.sh to check GLOBAL_CLIENT_VERSION before shell/environment variables
• Updated precedence documentation in comments to reflect: global flag → environment → spec option → default
• Updated test cases in tests/cli.bats to verify the new precedence order with explicit assertions for each level
• Updated README.md and ARCHITECTURE.md to document the corrected precedence

The change is minimal and surgical — only the order of two conditional branches in resolve_value() — with comprehensive test coverage confirming the new behavior at each precedence level.

https://claude.ai/code/session_0111eoWF5ABrxakfE2AbFsEk

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: acb7ffc9-be2a-4dfd-b71f-2e4432ebddb2

📥 Commits

Reviewing files that changed from the base of the PR and between 5aa6204 and 7afee90.

📒 Files selected for processing (2)

• qa-integration/pmm_qa/pmm-framework/lib/config.sh
• qa-integration/pmm_qa/pmm-framework/tests/cli.bats

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• percona/pmm-qa (manual)
• percona/pmm (manual)

💤 Files with no reviewable changes (2)

• qa-integration/pmm_qa/pmm-framework/lib/config.sh
• qa-integration/pmm_qa/pmm-framework/tests/cli.bats

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

The CLIENT_VERSION resolution order now gives priority to the global --client-version flag. The environment variable is evaluated next, followed by the database specification option and the registered default. The resolver comment, architecture documentation, and README describe this order. The CLI test verifies the updated precedence behavior and database specification fallback.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: the global flag now takes precedence over the environment variable.
Description check	✅ Passed	The description directly explains the precedence fix, affected files, tests, and documentation updates.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 04e5527b-f9a4-4cee-b1f3-28ba9ce27416

📥 Commits

Reviewing files that changed from the base of the PR and between 629b780 and 5aa6204.

📒 Files selected for processing (4)

• qa-integration/pmm_qa/pmm-framework/ARCHITECTURE.md
• qa-integration/pmm_qa/pmm-framework/README.md
• qa-integration/pmm_qa/pmm-framework/lib/config.sh
• qa-integration/pmm_qa/pmm-framework/tests/cli.bats

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• percona/pmm-qa (manual)
• percona/pmm (manual)

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

[claude]

No gate covers the behaviour this PR changes: Lint shellchecks lib/config.sh, but no workflow runs the pmm-framework bats suite (.claude/hooks/lint-changed.sh selects *.sh only, and nothing in .github/workflows invokes make check or bats for qa-integration/pmm_qa/pmm-framework), so the reordered tests/cli.bats assertions are unverified in CI. The body links a Claude session rather than a run proving those tests pass — please paste the local make check output, or the run, so the test claim is checkable.

[peterSirotnak]

Test evidence, run locally against the pushed head:

$ bats tests/*.bats
tests/cli.bats:        25 passed
tests/dispatch.bats:   14 passed
tests/integration.bats: 12 passed
tests/preflight.bats:    5 passed
# 56 passed, 0 failed
$ shellcheck lib/config.sh
# clean

Test 4, value precedence is global flag then environment then database then default, is the one that pins this change: resolve_value PS CLIENT_VERSION DB_CONFIG returns from-global while CLIENT_VERSION=from-env is set, then falls through to from-env once the flag is cleared, then to the spec value.

Agreed there's no CI gate running the pmm-framework bats suite today — Lint only shellchecks the *.sh files. That's a pre-existing gap and out of scope for this one-line precedence fix; worth a follow-up to wire bats qa-integration/pmm_qa/pmm-framework/tests into a workflow, but I'd rather not widen this PR to do it.

---

Generated by Claude Code