feat: interactive upgrade prompt for bare lark-cli - #1498
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds an interactive upgrade prompt for bare ChangesRoot Interactive Upgrade Prompt
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@d6a676e107565b435ee0ef37db3e9ec0902640ec🧩 Skill updatenpx skills add larksuite/cli#feat/root-interactive-upgrade -y -g |
c3d6bea to
d08f6b1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/root_upgrade_test.go (1)
101-108: ⚡ Quick winUse
cmdutil.TestFactory(t, config)instead of manualcmdutil.Factoryconstruction in tests.At Line 101 and Line 131, direct
&cmdutil.Factory{...}setup can drift from shared defaults and test harness behavior.As per coding guidelines,
**/*_test.go: “Usecmdutil.TestFactory(t, config)for test factories.”Also applies to: 131-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/root_upgrade_test.go` around lines 101 - 108, Replace the manual construction of cmdutil.Factory using &cmdutil.Factory{IOStreams: &cmdutil.IOStreams{...}} at both locations (around line 101 and line 131) with the cmdutil.TestFactory(t, config) helper function instead. This ensures the test factories use shared defaults and test harness behavior consistently across all tests, preventing drift from the intended test setup patterns.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/root_upgrade_test.go`:
- Around line 101-108: Replace the manual construction of cmdutil.Factory using
&cmdutil.Factory{IOStreams: &cmdutil.IOStreams{...}} at both locations (around
line 101 and line 131) with the cmdutil.TestFactory(t, config) helper function
instead. This ensures the test factories use shared defaults and test harness
behavior consistently across all tests, preventing drift from the intended test
setup patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5846f316-39d0-45df-a89a-7006e14656d2
📒 Files selected for processing (5)
cmd/build.gocmd/root_upgrade.gocmd/root_upgrade_test.gointernal/cmdutil/iostreams.gointernal/cmdutil/iostreams_test.go
d08f6b1 to
3e5a172
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1498 +/- ##
==========================================
+ Coverage 74.47% 74.48% +0.01%
==========================================
Files 849 850 +1
Lines 86631 86669 +38
==========================================
+ Hits 64517 64555 +38
Misses 17172 17172
Partials 4942 4942 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/root_upgrade.go`:
- Around line 22-26: Add test cases to cover the uncovered behavior branches
introduced in this PR. First, create a test for the runRootUpgrade function that
verifies the dispatch logic correctly finds and executes the "update" command
when it exists with a valid RunE function. Second, add a test case for the inner
== nil guard condition at line 82 to ensure the code handles the nil scenario
correctly. These tests should explicitly verify the expected behavior of each
branch to satisfy the requirement that every behavior change needs an
accompanying test.
🪄 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: 77eb891f-2862-473b-9c1d-5c78b671496e
📒 Files selected for processing (5)
cmd/build.gocmd/root_upgrade.gocmd/root_upgrade_test.gointernal/cmdutil/iostreams.gointernal/cmdutil/iostreams_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- cmd/build.go
- internal/cmdutil/iostreams.go
- cmd/root_upgrade_test.go
- internal/cmdutil/iostreams_test.go
3e5a172 to
876c35c
Compare
When a newer version is cached and stdin, stdout and stderr are all a TTY, bare `lark-cli` (no subcommand) prompts `Upgrade now? [y/N]` on stderr: `y` runs the existing `lark-cli update`, `n`/Enter/EOF skips, then the usual help prints. Only the bare root command triggers it; other commands, `lark-cli update`, and the JSON `_notice` channel are unchanged. Detection and throttle reuse update.CheckCached, with no new persisted state. - add cmd/root_upgrade.go (offerRootUpgrade, runRootUpgrade, readYes, isBareRootInvocation, installRootUpgradePrompt) - add OutIsTerminal to cmdutil.IOStreams; reuse existing StderrIsTerminal - wire installRootUpgradePrompt into cmd/build.go - add unit tests for the prompt logic and the new IOStreams fields
The test builds a Factory literal without going through NewDefault, so it never runs workspace detection and would otherwise inherit whatever value a prior test left in the process-global core.currentWorkspace. A leaked non-local workspace makes statePath() resolve to a subdir under LARKSUITE_CLI_CONFIG_DIR, so the written update-state.json is not found and the wantPrompt cases fail. Pin CurrentWorkspace to Local with restore, matching cmd/config/bind_test.go and cmd/auth/list_test.go.
b0dfca5 to
d6a676e
Compare
Summary
lark-cli already detects new versions, but the upgrade info is only surfaced via the JSON
_notice.updatefield, which is consumed by AI agents — a human typing commands in a terminal never sees it. This PR adds an interactive upgrade prompt for the barelark-cliinvocation (no subcommand): when a newer version is cached and the session is a real interactive terminal, it asksUpgrade now? [y/N]on stderr —yruns the existinglark-cli update,n/Enter/EOF skips — then prints the usual help. Only the bare root command triggers it; all other commands,lark-cli updateitself, and the JSON_noticechannel are unchanged.Changes
cmd/root_upgrade.go:offerRootUpgrade(three-stream TTY gate + cached-version check +[y/N]prompt),runRootUpgrade(reuses the registeredupdatesubcommand'sRunE),readYes,isBareRootInvocation, andinstallRootUpgradePromptOutIsTerminaltocmdutil.IOStreamsand reuse the existingStderrIsTerminalininternal/cmdutil/iostreams.go; the interactive gate requires stdin && stdout && stderr to all be a TTYinstallRootUpgradePrompt(f, rootCmd)after the unknown-subcommand guard incmd/build.goupdate.CheckCached;internal/update/update.gois unchanged — no new persisted statecmd/root_upgrade_test.goandinternal/cmdutil/iostreams_test.goTest Plan
make unit-testpassed_noticeunchanged, pipedynot consumed) are covered by acceptance-reviewer against a real release binary and by host-level E2E. skillave N/A (no shortcut/skill/meta change)lark-cli(non-TTY, cached newer version) prints help, exits 0, noUpgrade now?line;update-state.jsonbyte-identical before/after (no state written); JSON_notice.updateunchanged on subcommandsRelated Issues
N/A
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests