fix(cmdutil): default flag completions to disabled

[tuxedomm]

Summary

• The zero value of flagCompletionsDisabled was false (i.e. "enabled"), so any *cobra.Command built before configureFlagCompletions ran got registered into cobra's process-global flagCompletionFunctions map and never freed. scripts/bench_build measured hundreds of KB and thousands of retained objects per cmd.Build.
• Flip the semantics so the zero value is the safe default: rename the internal var to flagCompletionsEnabled (zero = disabled) and the public API to SetFlagCompletionsEnabled / FlagCompletionsEnabled. No init() needed. The API is internal/-only, so the rename blast radius is contained.
• Update the corresponding call sites in cmd/root.go::configureFlagCompletions, scripts/bench_build/main.go, and the affected tests.
• Add cmd.TestBuild_DefaultNoCompletionLeak: with no setter call at all, run cmd.Build 20 times and assert ≤ 50 KB / 500 objects per build (observed: ~0.7 KB / 3 objs). This closes the gap that let the wrong default ship — every previous test set the switch explicitly before exercising it, so the default value itself was never covered.

Test plan

• go test -race ./internal/cmdutil/ ./cmd/ ./shortcuts/common/
• go test -run TestBuild_DefaultNoCompletionLeak -v ./cmd/
• go build ./...

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR inverts the flag-completion control API from a negative ("disabled") model to a positive ("enabled") model, replacing SetFlagCompletionsDisabled/FlagCompletionsDisabled with SetFlagCompletionsEnabled/FlagCompletionsEnabled throughout the codebase. Behavior remains equivalent while using the affirmative API. A memory-leak test is added to verify default command construction doesn't retain memory.

Changes

Cohort / File(s)	Summary
Completion API Inversion internal/cmdutil/completion.go	Replaced SetFlagCompletionsDisabled/FlagCompletionsDisabled with SetFlagCompletionsEnabled/FlagCompletionsEnabled. Inverted registration logic to return early unless completions are enabled. Internal atomic variable renamed from flagCompletionsDisabled to flagCompletionsEnabled.
Command Root Configuration cmd/root.go	Updated configureFlagCompletions to use positive SetFlagCompletionsEnabled(completion) instead of SetFlagCompletionsDisabled(!completion).
Test Updates - API Migration cmd/root_test.go, internal/cmdutil/completion_test.go, shortcuts/common/runner_flag_completion_test.go	Migrated all tests to use the new enabled-based API. Test assertions updated to check enabled state rather than disabled state, with appropriate logic inversions.
Memory Leak Detection cmd/build_memstats_test.go	Added TestBuild_DefaultNoCompletionLeak to verify repeated command construction does not leak memory (heap growth < 50 KB, object growth < 500 per iteration) when completions are disabled.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix: skip flag-completion registration outside completion path #598: Directly related—introduces the original disabled-based flag-completion control API that this PR inverts to an enabled-based form.

Suggested labels

size/L

Suggested reviewers

• liangshuo-1

Poem

🐰 From "not disabled" to "enabled" we spring,
A logical leap on a semantic wing,
The API now gleams in affirmative light,
While memory tests keep allocations tight! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: inverting flag completions to disabled by default, which is the primary fix addressing the memory leak issue.
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.
Description check	✅ Passed	The PR description provides comprehensive coverage of the motivation, changes, test plan, and related issues.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/flag-completions-default-disabled

---

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 and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.92%. Comparing base (4d4508d) to head (ba30be7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #688   +/-   ##
=======================================
  Coverage   62.92%   62.92%           
=======================================
  Files         484      484           
  Lines       41522    41522           
=======================================
  Hits        26127    26127           
  Misses      13117    13117           
  Partials     2278     2278           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/build_memstats_test.go`:
- Around line 22-25: In TestBuild_DefaultNoCompletionLeak, make the test isolate
CLI config state by creating a temporary directory (via os.MkdirTemp), setting
the environment variable LARKSUITE_CLI_CONFIG_DIR to that temp dir with
os.Setenv before calling cmdutil.FlagCompletionsEnabled(), and defer restoring
the previous LARKSUITE_CLI_CONFIG_DIR value and removing the temp dir; this
ensures FlagCompletionsEnabled() observes an isolated config and keeps memstats
deterministic.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 100f303c-58c0-4e7d-95e0-0cb569858780

📥 Commits

Reviewing files that changed from the base of the PR and between 4d4508d and ba30be7.

📒 Files selected for processing (6)

• cmd/build_memstats_test.go
• cmd/root.go
• cmd/root_test.go
• internal/cmdutil/completion.go
• internal/cmdutil/completion_test.go
• shortcuts/common/runner_flag_completion_test.go

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ba30be788c5e3a5d4beb8ba93997b1eb4506e92a

🧩 Skill update

npx skills add larksuite/cli#fix/flag-completions-default-disabled -y -g