Ensure extension update notices only notify once within 24 hours, provide ability to disable all extension update notices #9934

andyfeller · 2024-11-18T03:06:28Z

This PR brings the new extension update checking logic up to par with what is done for gh by allowing users to disable notices via GH_NO_EXTENSION_UPDATE_NOTIFIER environment variable while skipping them in various unattended scenarios. The result being gh will only check only it has been 24 hours since the executed extension was used.

Testing these changes is a bit tricky as there is a degree of complexity with the update package that is difficult to test again due to ShouldCheckForExtensionUpdate() logic looking at stdout and stdin.

In addition to the automated tests, I also ran these changes through acceptance tests:

$ set -o pipefail && GH_ACCEPTANCE_TOKEN=$(gh auth token --hostname github.com) GH_ACCEPTANCE_HOST=github.com GH_ACCEPTANCE_ORG=gh-acceptance-testing go test -tags acceptance -json -run ^TestExtensions$ github.com/cli/cli/v2/acceptance  | tparse --all go test 
┌──────────────────────────────────────────────────────────────────────────────────┐
│  STATUS │ ELAPSED │           TEST           │             PACKAGE               │
│─────────┼─────────┼──────────────────────────┼───────────────────────────────────│
│  PASS   │   18.29 │ TestExtensions/extension │ github.com/cli/cli/v2/acceptance  │
│  PASS   │    0.00 │ TestExtensions           │ github.com/cli/cli/v2/acceptance  │
└──────────────────────────────────────────────────────────────────────────────────┘
┌────────────────────────────────────────────────────────────────────────────────────┐
│  STATUS │ ELAPSED │             PACKAGE              │ COVER │ PASS │ FAIL │ SKIP  │
│─────────┼─────────┼──────────────────────────────────┼───────┼──────┼──────┼───────│
│  PASS   │ 18.82s  │ github.com/cli/cli/v2/acceptance │  --   │  2   │  0   │  0    │
└────────────────────────────────────────────────────────────────────────────────────┘

Demo!

Made with vhs

For more information, see https://github.com/charmbracelet/vhs/

Set Shell zsh
Output extension-updates.gif
Set Width 1200
Set Height 600

# Setup
Hide
Type "gh ext remove eco"
Enter
Type "rm -rf ~/.local/state/gh/extensions"
Enter
Type "clear"
Enter
Show

# Demo older non-local extension version triggering update notice
Type "gh notifies user of newer releases when dispatching extensions"
Sleep 1s
Ctrl+U 

Type "./bin/gh ext install jrnxf/gh-eco --pin v0.1.4"
Enter
Sleep 3s

Type "./bin/gh eco"
Enter
Sleep 1s

Type "andyfeller"
Enter
Sleep 2s
Escape
Sleep 5s

# Demo extension metadata captured
Ctrl+L 
Type "gh captures metadata about extension updates"
Sleep 1s
Ctrl+U 

Type "tree ~/.local/state/gh"
Sleep 500ms
Enter

Type "cat ~/.local/state/gh/extensions/gh-eco/state.yml"
Enter
Sleep 5s

# Demo repeated non-local extension does not trigger update notice
Ctrl+L 
Type "gh only notifies once within 24 hours per extension"
Sleep 1s
Ctrl+U 

Type "./bin/gh eco"
Enter
Sleep 1s

Type "andyfeller"
Enter
Sleep 2s
Escape
Sleep 5s

# Demo removing extension clears metadata
Ctrl+L 
Type "gh cleans up metadata when removing extensions"
Sleep 1s
Ctrl+U 

Type "tree ~/.local/state/gh"
Sleep 500ms
Enter

Type "./bin/gh ext remove eco"
Sleep 500ms
Enter

Type "tree ~/.local/state/gh"
Enter
Sleep 5s

# Demo notifications can be disabled via environment variable
Ctrl+L 
Type "users can disable this with environment variable"
Sleep 1s
Ctrl+U 

Type "./bin/gh ext install jrnxf/gh-eco --pin v0.1.4"
Enter
Sleep 3s

Type "GH_NO_UPDATE_NOTIFIER=1 ./bin/gh eco"
Enter
Sleep 1s

Type "andyfeller"
Enter
Sleep 1s
Escape

Type "tree ~/.local/state/gh"
Enter
Sleep 5s

# Demo how metadata is removed when installing extensions
Ctrl+L 
Type "gh cleans up metadata when installing extensions"
Sleep 1s
Ctrl+U 

Type "./bin/gh eco"
Enter
Sleep 1s

Type "andyfeller"
Enter
Sleep 1s
Escape

Type "tree ~/.local/state/gh"
Sleep 500ms
Enter

Type "rm -rf ~/.local/share/gh/extensions/gh-eco"
Sleep 500ms
Enter

Type "./bin/gh ext install jrnxf/gh-eco --pin v0.1.4"
Sleep 500ms
Enter

Type "tree ~/.local/state/gh"
Sleep 500ms
Enter
Sleep 5s

This commit is a bit of refactoring to bring the extension update checking logic up to par with what is done with `gh` including creation of state file per extension and listening to env vars for disabling version checking. This work is not complete as it does not address necessary test changes.

This commit allows for stubbing/mocking the function used for checking update release info. Additionally, this does a minor refactor moving from non-blocking to blocking channel.

internal/update/update.go

First pass at implementing basic test around extension state checking behavior, wanting to discus with team about level of testing to perform and whether this is really the right place.

andyfeller · 2024-12-01T23:25:34Z

After discussing the state of this PR with @BagToad last week, I've been working on the tests around some of the changes as I'd like to see more of the extension update logic tested. I opened #9985 for discussion because the original logic is not really tested and I'm unsure about the level of value in testing at a particular level or not.

- extension manager has been made responsible for extension update directory information - extension manager has been enhanced to ensure that previously left extension update metadata is deleted before installing an extension - extension manager has been enhanced to ensure that extension update metadata is deleted when extension is being removed - refactored extension command tests for manager change, ensuring previous and expected states along with returned release info - refactored extension manager tests for ensuring previous extension update entries are removed before installing extension - created extension manager test for installing local extension - centralized logic for checking and ensuring extension name is "gh-" prefixed

- this was something I did in the original implementation of this improved extension update notification logic - discovering other parts of the extension manager code that were working with non-prefixed extension names motivated a different approach - the extension manager code that requires the extension be prefixed has been enhanced to use the centralized ensurePrefixed() logic, making the need for this on the extension unnecessary

- Local extensions on Windows are not symlinks but files whose contents is the path to the local extension directory - Local extensions elsewhere are symlinks

BagToad

Discussed with @andyfeller

internal/update/update.go

pkg/cmd/root/extension.go

internal/update/update.go

pkg/cmd/extension/manager.go

pkg/cmd/extension/manager_other_test.go

pkg/cmd/root/extension_test.go

After merging in upstream changes regarding local extensions, this commit addresses remaining PR feedback while also bringing the newly merged tests into alignment with other changes.

Extensions have to be gh- prefixed and these newer tests were failing because of expectation.

internal/ghcmd/cmd.go

pkg/cmd/extension/manager_test.go

andyfeller · 2024-12-14T04:42:02Z

pkg/cmd/root/extension.go

-
-func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension) *cobra.Command {
-	updateMessageChan := make(chan *extensionReleaseInfo)
+func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension, checkExtensionReleaseInfo func(extensions.ExtensionManager, extensions.Extension) (*update.ReleaseInfo, error)) *cobra.Command {


Open to ideas on alternatives to checkExtensionReleaseInfo function approach to make this testable. 🤷

Seems fine to me.

internal/update/update.go

williammartin · 2025-01-09T14:24:32Z

pkg/cmd/extension/manager.go

 	return cmd.Run()
 }
+
+// cleanExtensionUpdateDir deletes extension state directory to avoid problems reinstalling extensions.


@andyfeller can you list specific problems that you have in mind here? I can't think of any so I'm not sure I'm missing something.

Absolutely!

Normally, we expect users are removing extensions using gh ext remove <extension>, however I've heard people mention they or others can just rm -rf ~/.local/share/gh/extensions or remove the specific extension subdirectory.

If a user did this but did not remove the extension update state information in ~/.local/state/gh/extensions/<extension>, then extension manager can get into a weird state given extensions are not globally unique.

For example,

gh ext install andyfeller/gh-ext

rm -rf ~/.local/share/gh/extensions/gh-ext

gh ext install williammartin/gh-ext

Are these forks? Just coincidentally named extensions? What should the extension manager do if the old state was left around?

Instead, cleanExtensionUpdateDir is intended to clean up any lingering information to ensure installing an extension has a clean slate.

Thanks for the explanation. Does this have any actual practical implications right now?

I mean, if it's been less than 24 hours, we don't show the update notice. If it has been more than 24 hours we fetch the information for williammartin/gh-ext?

PS: cleanExtensionUpdateDir is used by the manager when installing or removing an extension.

Removing an extension

cli/pkg/cmd/extension/manager.go

Lines 571 to 584 in 6bbd703

func (m *Manager) Remove(name string) error {

name = normalizeExtension(name)

targetDir := filepath.Join(m.installDir(), name)

if _, err := os.Lstat(targetDir); os.IsNotExist(err) {

return fmt.Errorf("no extension found: %q", targetDir)

}

if m.dryRunMode {

return nil

}

if err := m.cleanExtensionUpdateDir(name); err != nil {

return err

}

return os.RemoveAll(targetDir)

}

Installing a local extension

cli/pkg/cmd/extension/manager.go

Lines 207 to 212 in 6bbd703

func (m *Manager) InstallLocal(dir string) error {

name := filepath.Base(dir)

if err := m.cleanExtensionUpdateDir(name); err != nil {

return err

}

targetLink := filepath.Join(m.installDir(), name)

Installing a binary extension

cli/pkg/cmd/extension/manager.go

Lines 334 to 343 in 6bbd703

// TODO clean this up if function errs?

if !m.dryRunMode {

if err := m.cleanExtensionUpdateDir(name); err != nil {

return err

}

if err = os.MkdirAll(targetDir, 0755); err != nil {

return fmt.Errorf("failed to create installation directory: %w", err)

}

}

Installing a script extension

cli/pkg/cmd/extension/manager.go

Lines 410 to 428 in 6bbd703

func (m *Manager) installGit(repo ghrepo.Interface, target string) error {

protocol := m.config.GitProtocol(repo.RepoHost()).Value

cloneURL := ghrepo.FormatRemoteURL(repo, protocol)

var commitSHA string

if target != "" {

var err error

commitSHA, err = fetchCommitSHA(m.client, repo, target)

if err != nil {

return err

}

}

name := strings.TrimSuffix(path.Base(cloneURL), ".git")

targetDir := filepath.Join(m.installDir(), name)

if err := m.cleanExtensionUpdateDir(name); err != nil {

return err

}

Nudge on my previous message because you might have missed it, commenting right after me.

Those are all completely legitimate concerns! 💯

Lastly, I thought this was a scenario you yourself mentioned when I first begun on this work.

Right, I mentioned that it would be nice if they could live in the extension installation location so that they got cleaned up automatically (I understand why that wasn't such an easy solution)

😞 Definitely a nuanced and difficult situation for several reasons:

Script based extensions are locally cloned git repositories, so adding this metadata there introduces some unknown behaviors

Local extensions don't support current/latest versions and work differently on Windows (file containing a path to the local extension) vs others (symlink to directory)

Hosting this state in a place where an extension author could collide or override it seems generally bad

Aligning this to XDG standard feels like this is state information that should be outside of the extension code

is there a concern motivating this question?

Yeh, three:

I'm concerned I don't understand if there is a change in behaviour that would occur by leaving these around.

I'm concerned that I, or someone else will have the same question, only less context in the future, so I'd like any specific changes in behaviour are captured in a comment for the future rather than non-specific "problems". If there are no specific changes in behaviour, and we choose to keep it, I want that to be expressed in the comment.

I'm generally just a bit concerned about the maintainability of a solution that requires cleanExtensionUpdateDir to be placed in three different locations in the three install functions, and in ways that aren't necessarily enforced by tests. The extra cognitive load understanding how the positioning of these calls relates to the code around them creates some friction (e.g. does it functionally matter if the call is inside the dryRun check block? Probably not, but I have to reason that out by understanding dryRun is currently only relevant for upgrades).

All-in-all, I think it's generally hard to reason about state management, and it only gets harder over time. I'm not really sure how to better express 3, other than it gives me the heebie-jeebies based on some reason I'm not skilled enough to articulate. Heebie-jeebies is non-blocking but it's enough to make me want 1 and 2 addressed.

Again, all completely valid points.

What else can we do to ensure we're confident about this given the efforts to consolidate related code into update package and incorporate extension update state in manager_test.go cases?

I'm genuinely asking as I want to ensure we do know and have confidence. 🙇

I can abide 🤜 🤛

Is this a limitation of the current extension architecture, current extension tests suite, or simply additional tests outside of the enhancements made to the install tests so far?

I can certainly :nod: that we should spend some time re-evaluating the extension architecture (manager, kind-specific types, manifests, etc) especially with an eye for testability and maintainability.

So I'd appreciate anything I can do to ensure our confidence.

By leaving this information around after removing an extension, it will either 1) lead to an immediate check because the last check time in the file is quite a bit older or 2) manual modifications to this file setting this date into the future would prevent notifications from going out.

This doesn't seem any different than installation of an extension for the first time, upon which we are going to do a check for an update immediately (which by the way, might be a bit annoying if I specifically pinned an extension on install, but isn't the end of the world).

What am I missing?

You're absolutely correct; this would be no different than if we install an extension for the first time. 👍 So, it isn't a problem.

The problem/concern is #2 where an older metadata file where the last checked date was manually set into the future that prevents the user from getting a notice. That said, as we're discussing this together at the time of writing, this is something we have no control over a user or a nefarious user doing intentionally.

Having discussed this further with @williammartin, I'm going to expand the documentation of this function to capture the expectations discussed here for posterity.

@williammartin : will defer to you for resolving this conversation given 0a1c9be as I want to ensure you are satisfied 🙇

LGTM. Though I'd maybe say "the only effect" -> "the only known effect"

internal/ghcmd/cmd.go

internal/update/update.go

pkg/cmd/extension/manager.go

williammartin · 2025-01-10T15:23:42Z

This is non blocking because I really want to get this PR shipped and it all relates to code structure.

As I looked at this PR again, I was interested in the "ownership" of the UpdateDir. Ostensibly, the ExtensionManager owns it, returning the filepath to it from the UpdateDir method. It also cleans it up on install and remove. However, it doesn't write to it, instead letting others do that (i.e. the update.go functions).

Therefore the contract is:

ExtensionManager owns the lifecycle of the dir
ExtensionManager allows for others to interact with the contents of the dir

This is alright, but in practice, the only collaborator for the second point is the update.go functions. It might make more sense in a follow up to bring the update logic into the ExtensionManager itself, so that the cobra lifecycle can call something like em.CheckForUpdate(ext).

I understand that currently the desire is to have parallels between the gh updating, and the ext manager updating logic. I believe that we could abstract a common module to be shared by these two updating-aware features, and have it called from ExtensionManager.

@williammartin

This commit expands the in-line docs around updaterEnabled package variable used to affect release checking. Along with clarifying specific details discovered when talking with @williammartin, I'm also removing a useless local variable.

During discussion in #9934, we can to the conclusion that the logic around checking for core GitHub CLI updates would diverge from GitHub CLI extension updates over time. To that end, this commit splits that logic into a separate function with a new environment variable.

This function was doing some unnecessary heavy lifting detecting if the directory being deleted actually existed when `os.RemoveAll()` would handle directories that exist or not.

This summarizes a conversation within #9934 PR regarding how the new extension update directory metadata would affect gh if not cleaned up.

BagToad

Tentatively approving this PR but @andyfeller and I want to give time for @williammartin to respond to any remaining comments before merging 🙂

Edit: thank you @andyfeller for the title edit ✨

pkg/cmd/root/help_topic.go

williammartin · 2025-01-13T12:45:14Z

pkg/cmd/root/help_topic.go

+			By default, gh checks for new releases once every 24 hours and displays an upgrade notice on standard
+			error if a newer version was found.
+
+			%[1]sGH_NO_EXTENSION_UPDATE_NOTIFIER%[1]s: set to any value to disable GitHub CLI extension update notifications.


Also no file changed here for me to comment on but this currently looks like:

A new release of sonar is available: 81a4ce86e027f31d306883c25a71b5d05b007e2e → 9977d8e535f98d57b15f0b5cb1dc887205acd2e4 To upgrade, run: gh extension upgrade sonar https://github.com/andyfeller/gh-sonar.git

"release" is maybe not the most accurate term for this.

It does make me think that the current upgrade approach around GitKind extensions is probably not very friendly, with a potential notification every 24 hours for any new git commits, and no obvious way to understand whether the git commit is interesting. Non-blocking.

williammartin

I have a few non-blocking notes.

This makes a significant difference to the latency when I'm running gh ext --help, thanks.

➜  cli git:(andyfeller/cli-cli-8183-timeout-pt2) hyperfine -w2 "gh sarif --help" "/Users/williammartin/workspace/cli/bin/gh sarif --help" "GH_NO_EXTENSION_UPDATE_NOTIFIER=true /Users/williammartin/workspace/cli/bin/gh sarif --help"
Benchmark 1: gh sarif --help
  Time (mean ± σ):      1.087 s ±  0.006 s    [User: 0.078 s, System: 0.026 s]
  Range (min … max):    1.073 s …  1.092 s    10 runs

Benchmark 2: /Users/williammartin/workspace/cli/bin/gh sarif --help
  Time (mean ± σ):      62.4 ms ±   4.7 ms    [User: 54.0 ms, System: 10.6 ms]
  Range (min … max):    58.6 ms …  76.4 ms    40 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 3: GH_NO_EXTENSION_UPDATE_NOTIFIER=true /Users/williammartin/workspace/cli/bin/gh sarif --help
  Time (mean ± σ):      62.6 ms ±   5.6 ms    [User: 54.4 ms, System: 10.6 ms]
  Range (min … max):    58.5 ms …  92.2 ms    47 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Summary
  /Users/williammartin/workspace/cli/bin/gh sarif --help ran
    1.00 ± 0.12 times faster than GH_NO_EXTENSION_UPDATE_NOTIFIER=true /Users/williammartin/workspace/cli/bin/gh sarif --help
   17.41 ± 1.30 times faster than gh sarif --help

- update `gh help environment` to include that upgrade notices only happen when gh or extensions are executed - update `gh ext --help` to include reference to upgrade notices and points to `gh help environment` for info on disabling

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.65.0` -> `v2.66.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.66.1`](https://github.com/cli/cli/releases/tag/v2.66.1): GitHub CLI 2.66.1 [Compare Source](cli/cli@v2.66.0...v2.66.1) #### Hotfix: `gh pr view` fails with provided URL This addresses a regression in `gh pr view` was reported in [#10352](cli/cli#10352). This regression was due to a change in `v2.66.0` that no longer allowed `gh pr` subcommands to execute properly outside of a git repo. #### What's Changed - Hotfix: `gh pr view` fails with provided URL by [@jtmcg](https://github.com/jtmcg) in cli/cli#10354 **Full Changelog**: cli/cli@v2.66.0...v2.66.1 ### [`v2.66.0`](https://github.com/cli/cli/releases/tag/v2.66.0): GitHub CLI 2.66.0 [Compare Source](cli/cli@v2.65.0...v2.66.0) #### `gh pr view` and `gh pr status` now respect common triangular workflow configurations Previously, `gh pr view` and `gh pr status` would fail for pull request's (MR) open in triangular workflows. This was due to `gh` being unable to identify the MR's corresponding remote and branch refs on GitHub. Now, `gh pr view` and `gh pr status` should successfully identify the MR's refs when the following common git configurations are used: - [`branch.<branchName>.pushremote`](https://git-scm.com/docs/git-config#Documentation/git-config.txt-branchltnamegtpushRemote) is set - [`remote.pushDefault`](https://git-scm.com/docs/git-config#Documentation/git-config.txt-remotepushDefault) is set Branch specific configuration, the former, supersedes repo specific configuration, the latter. Additionally, if the [`@{push}` revision syntax](https://git-scm.com/docs/gitrevisions#Documentation/gitrevisions.txt-emltbranchnamegtpushemegemmasterpushemempushem) for git resolves for a branch, `gh pr view` and `gh pr status` should work regardless of additional config settings. For more information, see - cli/cli#9363 - cli/cli#9364 - cli/cli#9365 - cli/cli#9374 #### `gh secret list`, `gh secret set`, and `gh secret delete` now require repository selection when multiple `git` remotes are present Previously, `gh secret list`, `gh secret set`, and `gh secret delete` would determine which remote to target for interacting with GitHub Actions secrets. Remotes marked as default using `gh repo set-default` or through other `gh` commands had higher priority when figuring out which repository to interact with. This could have unexpected outcomes when using `gh secret` commands with forked repositories as the upstream repository would generally be selected. Now, `gh secret` commands require users to disambiguate which repository should be the target if multiple remotes are present and the `-R, --repo` flag is not provided. For more information, see cli/cli#4688 #### Extension update notices now notify once every 24 hours per extension and can be disabled Previously, the GitHub CLI would notify users about newer versions every time an extension was executed. This did not match GitHub CLI notices, which only notified users once every 24 hours and could be disabled through an environment variable. Now, extension update notices will behave similar to GitHub CLI notices. To disable extension update notices, set the `GH_NO_EXTENSION_UPDATE_NOTIFIER` environment variable. For more information, see cli/cli#9925 #### What's Changed ##### ✨ Features - Draft for discussing testing around extension update checking behavior by [@andyfeller](https://github.com/andyfeller) in cli/cli#9985 - Make extension update check non-blocking by [@andyfeller](https://github.com/andyfeller) in cli/cli#10239 - Ensure extension update notices only notify once within 24 hours, provide ability to disable all extension update notices by [@andyfeller](https://github.com/andyfeller) in cli/cli#9934 - feat: make the extension upgrade fancier by [@nobe4](https://github.com/nobe4) in cli/cli#10194 - fix: padded display by [@nobe4](https://github.com/nobe4) in cli/cli#10216 - Update `gh attestation` attestation bundle fetching logic by [@malancas](https://github.com/malancas) in cli/cli#10185 - Require repo disambiguation for secret commands by [@williammartin](https://github.com/williammartin) in cli/cli#10209 - show error message for rerun workflow older than a month ago by [@iamrajhans](https://github.com/iamrajhans) in cli/cli#10227 - Update `gh attestation verify` table output by [@malancas](https://github.com/malancas) in cli/cli#10104 - Enable MSI building for Windows arm64 by [@dennisameling](https://github.com/dennisameling) in cli/cli#10297 - feat: Add support for creating autolink references by [@hoffm](https://github.com/hoffm) in cli/cli#10180 - Find MRs using `@{push}` by [@Frederick888](https://github.com/Frederick888) in cli/cli#9208 - feat: Add support for viewing autolink references by [@hoffm](https://github.com/hoffm) in cli/cli#10324 - Update `gh attestation` bundle fetching logic by [@malancas](https://github.com/malancas) in cli/cli#10339 ##### 🐛 Fixes - gh gist delete: prompt for gist id by [@danochoa](https://github.com/danochoa) in cli/cli#10154 - Better handling for waiting for codespaces to become ready by [@cmbrose](https://github.com/cmbrose) in cli/cli#10198 - Fix: `gh gist view` and `gh gist edit` prompts with no TTY by [@mateusmarquezini](https://github.com/mateusmarquezini) in cli/cli#10048 - Remove naked return values from `ReadBranchConfig` and `prSelectorForCurrentBranch` by [@jtmcg](https://github.com/jtmcg) in cli/cli#10197 - Add job to deployment workflow to validate the tag name for a given release by [@jtmcg](https://github.com/jtmcg) in cli/cli#10121 - \[gh run list] Stop progress indicator on failure from `--workflow` flag by [@iamazeem](https://github.com/iamazeem) in cli/cli#10323 - Update deployment.yml by [@andyfeller](https://github.com/andyfeller) in cli/cli#10340 ##### 📚 Docs & Chores - Add affected version heading to bug report issue form by [@BagToad](https://github.com/BagToad) in cli/cli#10269 - chore: fix some comments by [@petercover](https://github.com/petercover) in cli/cli#10296 - Update triage.md to reflect FR experiment outcome by [@jtmcg](https://github.com/jtmcg) in cli/cli#10196 - Clear up --with-token fine grained PAT usage by [@williammartin](https://github.com/williammartin) in cli/cli#10186 - Correct help documentation around template use in `gh issue create` by [@andyfeller](https://github.com/andyfeller) in cli/cli#10208 - chore: fix some function names in comment by [@zhuhaicity](https://github.com/zhuhaicity) in cli/cli#10225 - Tiny typo fix by [@robmorgan](https://github.com/robmorgan) in cli/cli#10265 - add install instructions for Manjaro Linux by [@AMS21](https://github.com/AMS21) in cli/cli#10236 - Update test to be compatible with latest Glamour v0.8.0 by [@ottok](https://github.com/ottok) in cli/cli#10151 - Add more `gh attestation verify` integration tests by [@malancas](https://github.com/malancas) in cli/cli#10102 #####

Dependencies - Bump github.com/mattn/go-colorable from 0.1.13 to 0.1.14 by [@dependabot](https://github.com/dependabot) in cli/cli#10215 - Bump github.com/sigstore/protobuf-specs from 0.3.2 to 0.3.3 by [@dependabot](https://github.com/dependabot) in cli/cli#10214 - Bump github.com/gabriel-vasile/mimetype from 1.4.7 to 1.4.8 by [@dependabot](https://github.com/dependabot) in cli/cli#10184 - Bump google.golang.org/protobuf from 1.36.2 to 1.36.3 by [@dependabot](https://github.com/dependabot) in cli/cli#10250 - Bump golangci-linter and address failures to prepare for Go 1.24 strictness by [@mikelolasagasti](https://github.com/mikelolasagasti) in cli/cli#10279 - Bump github.com/google/go-containerregistry from 0.20.2 to 0.20.3 by [@dependabot](https://github.com/dependabot) in cli/cli#10257 - Bump actions/attest-build-provenance from 2.1.0 to 2.2.0 by [@dependabot](https://github.com/dependabot) in cli/cli#10300 - Bump google.golang.org/protobuf from 1.36.3 to 1.36.4 by [@dependabot](https://github.com/dependabot) in cli/cli#10306 - Upgrade sigstore-go to v0.7.0: fixes [#10114](cli/cli#10114) formatting issue by [@codysoyland](https://github.com/codysoyland) in cli/cli#10309 - Bump github.com/in-toto/attestation from 1.1.0 to 1.1.1 by [@dependabot](https://github.com/dependabot) in cli/cli#10319 #### New Contributors Big thank you to our many new *and* longtime contributors making this release happen!! ❤️ ✨ - [@zhuhaicity](https://github.com/zhuhaicity) made their first contribution in cli/cli#10225 - [@danochoa](https://github.com/danochoa) made their first contribution in cli/cli#10154 - [@robmorgan](https://github.com/robmorgan) made their first contribution in cli/cli#10265 - [@iamrajhans](https://github.com/iamrajhans) made their first contribution in cli/cli#10227 - [@AMS21](https://github.com/AMS21) made their first contribution in cli/cli#10236 - [@petercover](https://github.com/petercover) made their first contribution in cli/cli#10296 - [@ottok](https://github.com/ottok) made their first contribution in cli/cli#10151 - [@dennisameling](https://github.com/dennisameling) made their first contribution in cli/cli#10297 - [@iamazeem](https://github.com/iamazeem) made their first contribution in cli/cli#10323 - [@Frederick888](https://github.com/Frederick888) made their first contribution in cli/cli#9208 **Full Changelog**: cli/cli@v2.65.0...v2.66.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

During discussion in cli#9934, we can to the conclusion that the logic around checking for core GitHub CLI updates would diverge from GitHub CLI extension updates over time. To that end, this commit splits that logic into a separate function with a new environment variable.

This summarizes a conversation within cli#9934 PR regarding how the new extension update directory metadata would affect gh if not cleaned up.

andyfeller added 2 commits November 17, 2024 16:10

Refactor extension command testing

7b91b17

This commit allows for stubbing/mocking the function used for checking update release info. Additionally, this does a minor refactor moving from non-blocking to blocking channel.

andyfeller marked this pull request as ready for review November 22, 2024 21:28

andyfeller requested a review from a team as a code owner November 22, 2024 21:28

andyfeller requested a review from BagToad November 22, 2024 21:28

cliAutomation added the external pull request originating outside of the CLI core team label Nov 22, 2024

jtmcg mentioned this pull request Nov 22, 2024

Significant Zsh startup delay after adding gh copilot alias #9950

Open

BagToad reviewed Nov 22, 2024

View reviewed changes

internal/update/update.go Outdated Show resolved Hide resolved

Initial extension update check testing

6bd01d5

First pass at implementing basic test around extension state checking behavior, wanting to discus with team about level of testing to perform and whether this is really the right place.

Andrew Feller and others added 4 commits December 8, 2024 19:40

Test whether extension update should be checked

a79e086

Expand update tests for local extensions

10f62b1

andyfeller requested a review from BagToad December 9, 2024 02:10

andyfeller added 4 commits December 8, 2024 21:17

Remove unnecessary Extension.FullName()

a6b8bec

Refactor local installation manager test by OS

52b845b

- Local extensions on Windows are not symlinks but files whose contents is the path to the local extension directory - Local extensions elsewhere are symlinks

Add build constraint for non-Windows systems

56841d9

Add build constraint for non-Windows systems

c8501d8

BagToad requested changes Dec 13, 2024

View reviewed changes

andyfeller added 3 commits December 13, 2024 22:52

Implement partial feedback before trunk update

c12e369

Merge branch 'trunk' into andyfeller/cli-cli-8183-timeout-pt2

5ad6ccf

Implement remaining PR feedback

bfc63a1

After merging in upstream changes regarding local extensions, this commit addresses remaining PR feedback while also bringing the newly merged tests into alignment with other changes.

andyfeller requested a review from BagToad December 14, 2024 04:12

Fix failig local extension tests

edc0351

Extensions have to be gh- prefixed and these newer tests were failing because of expectation.

andyfeller commented Dec 14, 2024

View reviewed changes

internal/ghcmd/cmd.go Show resolved Hide resolved

andyfeller commented Dec 14, 2024

View reviewed changes

pkg/cmd/extension/manager_test.go Show resolved Hide resolved

andyfeller commented Dec 14, 2024

View reviewed changes

pkg/cmd/extension/manager_test.go Show resolved Hide resolved

andyfeller commented Dec 14, 2024

View reviewed changes

BagToad reviewed Jan 7, 2025

View reviewed changes

internal/update/update.go Outdated Show resolved Hide resolved

Comment exported functions

6bbd703

andyfeller requested a review from BagToad January 9, 2025 14:10

williammartin reviewed Jan 9, 2025

View reviewed changes

andyfeller commented Jan 10, 2025

View reviewed changes

internal/ghcmd/cmd.go Show resolved Hide resolved

andyfeller commented Jan 10, 2025

View reviewed changes

internal/update/update.go Outdated Show resolved Hide resolved

andyfeller commented Jan 10, 2025

View reviewed changes

pkg/cmd/extension/manager.go Outdated Show resolved Hide resolved

Collapse dryrun checks in ext bin upgrade

889c4c7

williammartin force-pushed the andyfeller/cli-cli-8183-timeout-pt2 branch from 6d33007 to 889c4c7 Compare January 10, 2025 15:06

andyfeller added 4 commits January 10, 2025 13:55

Capture greater detail on updaterEnabled

1c32c98

This commit expands the in-line docs around updaterEnabled package variable used to affect release checking. Along with clarifying specific details discovered when talking with @williammartin, I'm also removing a useless local variable.

Simplifying cleanExtensionUpdateDir logic

97eb8a1

This function was doing some unnecessary heavy lifting detecting if the directory being deleted actually existed when `os.RemoveAll()` would handle directories that exist or not.

Expand docs on cleaning extension update dir

0a1c9be

This summarizes a conversation within #9934 PR regarding how the new extension update directory metadata would affect gh if not cleaned up.

andyfeller changed the title ~~Bring extension update logic up to par with gh update logic~~ Ensure extension update notices only notify once within 24 hours, provide ability to disable all extension update notices Jan 10, 2025

BagToad approved these changes Jan 13, 2025

View reviewed changes

williammartin reviewed Jan 13, 2025

View reviewed changes

pkg/cmd/root/help_topic.go Show resolved Hide resolved

williammartin reviewed Jan 13, 2025

View reviewed changes

pkg/cmd/root/help_topic.go Show resolved Hide resolved

williammartin reviewed Jan 13, 2025

View reviewed changes

williammartin approved these changes Jan 13, 2025

View reviewed changes

Enhance help docs on ext upgrade notices

3ada5af

- update `gh help environment` to include that upgrade notices only happen when gh or extensions are executed - update `gh ext --help` to include reference to upgrade notices and points to `gh help environment` for info on disabling

andyfeller merged commit 41e99b9 into trunk Jan 13, 2025

andyfeller deleted the andyfeller/cli-cli-8183-timeout-pt2 branch January 13, 2025 14:12

andyfeller mentioned this pull request Jan 13, 2025

Extension update notices should be non-blocking #10235

Closed

rancorm pushed a commit to rancorm/cli that referenced this pull request May 3, 2025

Expand docs on cleaning extension update dir

63c5de3

This summarizes a conversation within cli#9934 PR regarding how the new extension update directory metadata would affect gh if not cleaned up.

	func (m *Manager) Remove(name string) error {
	name = normalizeExtension(name)
	targetDir := filepath.Join(m.installDir(), name)
	if _, err := os.Lstat(targetDir); os.IsNotExist(err) {
	return fmt.Errorf("no extension found: %q", targetDir)
	}
	if m.dryRunMode {
	return nil
	}
	if err := m.cleanExtensionUpdateDir(name); err != nil {
	return err
	}
	return os.RemoveAll(targetDir)
	}

	func (m *Manager) InstallLocal(dir string) error {
	name := filepath.Base(dir)
	if err := m.cleanExtensionUpdateDir(name); err != nil {
	return err
	}
	targetLink := filepath.Join(m.installDir(), name)

	// TODO clean this up if function errs?
	if !m.dryRunMode {
	if err := m.cleanExtensionUpdateDir(name); err != nil {
	return err
	}

	if err = os.MkdirAll(targetDir, 0755); err != nil {
	return fmt.Errorf("failed to create installation directory: %w", err)
	}
	}

	func (m *Manager) installGit(repo ghrepo.Interface, target string) error {
	protocol := m.config.GitProtocol(repo.RepoHost()).Value
	cloneURL := ghrepo.FormatRemoteURL(repo, protocol)

	var commitSHA string
	if target != "" {
	var err error
	commitSHA, err = fetchCommitSHA(m.client, repo, target)
	if err != nil {
	return err
	}
	}

	name := strings.TrimSuffix(path.Base(cloneURL), ".git")
	targetDir := filepath.Join(m.installDir(), name)

	if err := m.cleanExtensionUpdateDir(name); err != nil {
	return err
	}

Ensure extension update notices only notify once within 24 hours, provide ability to disable all extension update notices #9934

Ensure extension update notices only notify once within 24 hours, provide ability to disable all extension update notices #9934

Uh oh!

Conversation

andyfeller commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Demo!

Uh oh!

Uh oh!

andyfeller commented Dec 1, 2024

Uh oh!

BagToad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyfeller Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

williammartin commented Jan 10, 2025

Uh oh!

BagToad left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

williammartin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

andyfeller commented Nov 18, 2024 •

edited

Loading

andyfeller Jan 9, 2025 •

edited

Loading

BagToad left a comment •

edited

Loading

williammartin left a comment •

edited

Loading