feat(ux): add background update check with 24h cache#80
Conversation
212bb71 to
a93df63
Compare
KayIter
left a comment
There was a problem hiding this comment.
I reviewed this PR as a CLI behavior change. The update-check implementation is nicely isolated, but I found two issues worth addressing before merge.
Required
The background update check can still block normal command exit.
Lines 218 to 231 in a93df63
Execute() starts updatecheck.Start() before running the command, but after the command completes it calls updatecheck.PrintNotice(). PrintNotice() does a blocking receive from the channel:
ags-cli/internal/updatecheck/updatecheck.go
Lines 64 to 75 in a93df63
So on a cold/expired cache, if the VERSION endpoint is slow or unreachable, every ordinary interactive command can pause at exit until the HTTP request finishes, up to the current httpTimeout of 5 seconds. That contradicts the “never blocks the command” comment and makes a background UX feature visible as latency.
I reproduced this locally with an httptest server that sleeps 200ms before returning a version; PrintNotice blocked for ~201ms, matching the server delay.
Suggested fix: do not wait unconditionally in the foreground path. Use a non-blocking select and only print when the result is already ready, or wait with a very small budget, or only print cached results from a previous check and let the fresh network check update cache for the next invocation.
Suggested
The new root tests for agr update depend on the real public VERSION endpoint.
ags-cli/tests/root/update_test.go
Lines 19 to 40 in a93df63
agr update calls updatecheck.FetchLatestVersion() directly:
ags-cli/internal/cli/update.go
Lines 28 to 30 in a93df63
That makes the default root test suite depend on https://dl.tencentags.com/agr-cli/latest/VERSION and external network availability. If that service is slow, unavailable, or blocked in CI, these tests will fail even when the CLI code is fine.
Suggested fix: add a test-only/env URL override and point the root tests at an httptest server, or move the real endpoint check into an explicit live/integration suite.
Validation I ran locally:
go test ./internal/updatecheck ./internal/cli ./tests/root
Those packages pass; the first issue is a behavior/latency gap rather than a currently failing unit test.
a93df63 to
7f005c8
Compare
|
Two follow-up fixes for the update-notice timing/exit-path issues raised in review: 1. Warm-cache notice dropped on fast commands
Fix: 2. Error path bypassed PrintNotice via os.Exit
Fix: E2E verification (real binary, warm cache latest=v9.9.9)
Regression tests added: warm-cache result must be ready immediately; cold-cache must not block. |
5e992b4 to
ebcca50
Compare
KayIter
left a comment
There was a problem hiding this comment.
还有一个参数解析上的残留问题需要处理。
[P2] shouldRunUpdateCheck 只检查 args[0] 来判断是否跳过后台 update check,但全局 flags 可以出现在子命令前面。比如 agr --config custom.toml update 时 args[0] 是 --config,不会命中 update 的 skip 规则,于是会启动后台检查;update 命令本身随后又会执行前台 FetchLatestVersion,warm cache 下还可能额外打印一段 background update notice。类似地,agr -o json status 会在 JSON 模式下仍启动后台检查,只是最后不打印。
我用临时回归测试复现过:
if shouldRunUpdateCheck([]string{"--config", "custom.toml", "update"}) {
t.Fatal("expected update check to be skipped for update command after global flag")
}
applyRawGlobalArgs([]string{"-o", "json", "status"})
if shouldRunUpdateCheck([]string{"-o", "json", "status"}) {
t.Fatal("expected update check to be skipped for JSON output")
}建议先剥离/跳过全局 flags 后再判断真实 command token,或者复用现有的 raw args token extraction / effective output 判断,让注释里的 skip 规则真正生效。
本地验证:
go test ./internal/cli ./tests/root通过go test ./internal/updatecheck在允许 httptest 监听本地端口后通过
0296fd3 to
9c62ee7
Compare
Add non-blocking background version check for agr CLI, inspired by AgentCore CLI's update-notifier.ts. On every invocation a goroutine checks for new versions; the notice is printed to stderr after the command finishes without blocking execution. Key features: - Background goroutine + non-blocking select (never delays command exit) - 24h file cache (~/.agr/update-check.json) to avoid redundant HTTP calls - Semver comparison handling v-prefix, pre-release, and dev versions - 'agr update' explicit command with text/JSON output - Respects --non-interactive and -o json (suppresses notice) - Skips for version/help/update commands and --version/-v/--help/-h flags - extractCommandToken reuses extractCommandTokens for consistent flag parsing - Only scans for version/help flags when no subcommand found (avoids false positives from subcommand arguments) - FetchLatestVersion uses io.ReadAll+LimitReader for reliability - _TEST_VERSION_URL env var and CLI.ExtraEnv for offline E2E testing - Unit tests for shouldRunUpdateCheck/extractCommandToken (16 cases) - Offline E2E tests using httptest mock server Closes TencentCloudAgentRuntime#76
9c62ee7 to
fbde5df
Compare
KayIter
left a comment
There was a problem hiding this comment.
复查了最新提交 fbde5df,之前 request changes 的点已经修掉了。
这版 shouldRunUpdateCheck 不再只看 args[0],而是通过 extractCommandToken 识别真实子命令;--config ... update、-o json status 这类全局 flag 在前的场景也有了对应处理/覆盖。JSON 模式下也会直接跳过后台 update check,避免不必要的网络 I/O。
本地验证通过:
go test ./internal/cli ./internal/updatecheck ./tests/root
LGTM.
Summary
Add a non-blocking background version check inspired by AgentCore CLI's update-notifier.ts. Every
agrinvocation triggers a background goroutine that checks for newer versions. A notice is printed to stderr after the command finishes — it never blocks the actual command.Features
~/.agr/update-check.jsonwith{"last_check": unix_ts, "latest": "v0.7.0"}agr updatecommandagr update --check--non-interactive-o jsonagr update/agr version/agr helpdon't trigger background checkhttps://dl.tencentags.com/agr-cli/latest/VERSION(already exists)Expected Behavior
$ agr instance list ... normal output ... Update available: v0.6.2 → v0.7.0 Run: curl -fsSL https://dl.tencentags.com/agr-cli/latest/install.sh | sh$ agr update Current: v0.6.2 Latest: v0.7.0 Update available: v0.6.2 → v0.7.0 Run: curl -fsSL https://dl.tencentags.com/agr-cli/latest/install.sh | shDesign Constraints Met
--non-interactive-o json~/.agr/update-check.jsonhttps://dl.tencentags.com/agr-cli/latest/VERSIONagr updateitself skips the background check (no infinite loop)Files
New
internal/updatecheck/updatecheck.go— Core: Start(), PrintNotice(), CompareVersions(), FetchLatestVersion(), cache I/Ointernal/updatecheck/updatecheck_test.go— 11 unit testsinternal/cli/update.go—agr updatecommandModified
internal/cli/root.go— Integrate background check in Execute()Test Results
Closes #76