Conversation
Record successful explicit updates while preserving the Auto setting, and skip stale automatic updates when Auto is disabled. Default malformed Auto values to enabled for backward compatibility and leave config unchanged on failed updates. Ignore local AI tooling files.
|
Coverage Impact ⬆️ Merging this pull request will increase total coverage on Modified Files with Diff Coverage (2)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
Contributor
Test Results 4 files ± 0 24 suites ±0 1m 3s ⏱️ -6s Results for commit 256a16c. ± Comparison against base commit cf5551b. This pull request removes 1 and adds 13 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate issues remain unresolved in configuration handling, pack initialization, and update-date durability.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR adds UpdateDaily handling, records successful explicit index updates, and introduces update-index --daily.
Changes:
- Supports legacy configuration while adding
UpdateDaily. - Persists update dates after successful updates.
- Adds CLI and test coverage.
- Ignores local AI tooling files.
File summaries
| File | Summary |
|---|---|
cmd/installer/update_cfg_internal_test.go |
Tests configuration helpers. |
cmd/installer/root.go |
Implements configuration and automatic update behavior; unresolved preservation and durability issues remain. |
cmd/installer/root_test.go |
Tests automatic update scenarios. |
cmd/commands/update_index.go |
Adds CLI control and date recording; configuration-only updates need proper initialization and unlocking. |
cmd/commands/update_index_test.go |
Tests CLI update behavior. |
cmd/commands/update_index_internal_test.go |
Tests command validation. |
.gitignore |
Ignores local AI tooling files. |
Review details
Suppressed comments (2)
cmd/commands/update_index.go:46
- The
--dailypath only runsconfigureInstallerGlobalCmd, so it never initializes/unlocks the installer before writing.Web/update.cfg. For a pack root locked byLockPackRoot, a config-only invocation that needs to createupdate.cfg(and platforms that enforce directory read-only semantics for existing files) returns a permission error. Configure and unlock the pack root for this branch as well, then relock it on return.
if cmd.Flags().Changed("daily") {
if err := configureInstallerGlobalCmd(cmd, args); err != nil {
return err
}
return installer.SetUpdateDaily(viper.GetString("pack-root"), updateIndexCmdFlags.daily)
cmd/installer/root.go:989
- When the index file is missing, this branch ignores
update.cfgand unconditionally rewrites both settings astrue. A user who previously ranupdate-index --daily=false(or had legacyAuto=false) can therefore get an automatic bootstrap update that silently re-enables the setting the next time this helper runs. Preserve the existing configuration, or skip the automatic download whenUpdateDailyis disabled, instead of resetting these flags.
updateConf.Auto = true
updateConf.UpdateDaily = true
- Files reviewed: 6/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… closed without error handling' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
cpackget addcrashes when updating a filesystem hosted.Web/index.pidxgenerated byvidx2pidx#835update.cfgspecifiesUpdateDaily=false.Changes
update.cfgonly afterupdate-indexsucceeds, while preserving theAutosetting.Risks / Limitations
update.cfgfails after the index is updated,update-indexreturns an error even though the index update itself succeeded.Checklist