Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/379b893a-5227-4418-8333-ced63e46cbf1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/379b893a-5227-4418-8333-ced63e46cbf1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/379b893a-5227-4418-8333-ced63e46cbf1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/379b893a-5227-4418-8333-ced63e46cbf1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a compile-time background update check that uses unauthenticated GitHub release + raw-content endpoints to recommend upgrading the installed gh-aw compiler, with opt-out via GH_AW_DISABLE_UPDATE_CHECK.
Changes:
- Introduces a new async “compile update check” that discovers the latest release tag via
/releases/latest, probes tag existence viaraw.githubusercontent.com, and prints targeted notifications. - Wires the new check into the
compilecommand lifecycle (start at command run, print if ready before exit). - Adds unit tests covering enable/disable logic, version comparison behavior, and notification output.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/compile_update_check.go | New compile-only background update check logic + notification printing. |
| pkg/cli/compile_update_check_test.go | Unit tests for the compile update check and output formatting. |
| cmd/gh-aw/main.go | Replaces the compile command’s previous async update check call with the new compile update check start/finish flow. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 4
| // Drain a small amount so the response body is consumed before close and the | ||
| // underlying connection remains reusable. | ||
| _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 1024)) |
| // StartCompileUpdateCheck begins a best-effort update check for the compile command. | ||
| // The returned function should be called once before the command exits to print any | ||
| // ready notification without blocking compilation for long. | ||
| func StartCompileUpdateCheck(ctx context.Context, noCheckUpdate bool, verbose bool) func() { | ||
| if !shouldRunCompileUpdateCheck(noCheckUpdate) { | ||
| return func() {} | ||
| } | ||
|
|
| compileUpdateCheckDisableEnv = "GH_AW_DISABLE_UPDATE_CHECK" | ||
| compileUpdateCheckTimeout = 3 * time.Second | ||
| compileUpdateCheckWait = 500 * time.Millisecond | ||
| maxProbeFileSize = 64 * 1024 | ||
| ) |
| oldStderr := os.Stderr | ||
| r, w, err := os.Pipe() | ||
| require.NoError(t, err, "pipe creation should succeed") | ||
| os.Stderr = w | ||
|
|
||
| printCompileUpdateNotification(tt.notification) | ||
|
|
||
| require.NoError(t, w.Close(), "pipe writer should close cleanly") | ||
| os.Stderr = oldStderr | ||
|
|
||
| var buf bytes.Buffer | ||
| _, err = buf.ReadFrom(r) | ||
| require.NoError(t, err, "pipe reader should capture stderr output") | ||
| output := buf.String() |
|
@copilot merge main and recompile |
|
@copilot review existing projects that perform this type of background check and apply best practices to keep it non-invasive, resilient to connectivity issues, and efficient on low bandwidth. Aim for the best possible implementation. There are dedicated projects in npm that handle this, and there may be options in Go as well. |
…e-command-background-check # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3c5cce76-f3d9-49b8-9c7b-9e396b86a7e2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3c5cce76-f3d9-49b8-9c7b-9e396b86a7e2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3c5cce76-f3d9-49b8-9c7b-9e396b86a7e2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3c5cce76-f3d9-49b8-9c7b-9e396b86a7e2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3c5cce76-f3d9-49b8-9c7b-9e396b86a7e2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented the notifier-style hardening in cd1b1b1: the compile check now throttles with a persisted daily marker, uses non-blocking shutdown behavior, switches release/probe lookups to HEAD for lower bandwidth, and adds focused coverage for the new behavior. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
Agent-Logs-Ur-ifaceassert` (http block)
|
Reviewed all current PR comments. The outstanding reviewer points are already covered in cd1b1b1: the check is throttled with a persisted daily marker, shutdown notification printing is non-blocking, release/probe lookups use HEAD, and the test now closes the pipe reader. I did not find any additional code changes needed. |
|
@copilot review the implementation to ensure it follows best practices for notification/background upgrade-checks. Confirm it does not introduce performance regressions, does not block compile, and works correctly in lockdown environments. |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/32665f07-3076-40eb-806b-08c6fcf14638 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/32665f07-3076-40eb-806b-08c6fcf14638 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/32665f07-3076-40eb-806b-08c6fcf14638 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/32665f07-3076-40eb-806b-08c6fcf14638 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/32665f07-3076-40eb-806b-08c6fcf14638 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/32665f07-3076-40eb-806b-08c6fcf14638 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reviewed and tightened the implementation in 7a04b86. The compile-only notifier now skips headless/non-interactive sessions, keeps the shutdown path non-blocking, and has synchronized tests proving locked-down network failures stay silent without delaying compile. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
Agent-Logs-Url: https://git-U0 modules/@npmcli/run-script/lib/node-gyp-bin/node --show-toplevel git 64/bin/node git` (http block)
Agent-Logs-Url: htt-U0 /usr/bin/git --show-toplevel git /usr/bin/infocmp--show-toplevel git ode_�� 88ed8e6a:pkg/cli/compile_update_check_test.go infocmp x86_64/sh xterm-color gh /usr/bin/git gh` (http block)
|
Summary
HEADrequests for release/probe lookups, and skipping notifications in headless/non-interactive sessionsGH_AW_DISABLE_UPDATE_CHECKand add focused unit/integration coverageValidation
go test -v -run 'TestShouldRunCompileUpdateCheck|TestRunCompileUpdateCheck|TestRunCompileUpdateCheckUsesHEADRequests|TestPrintCompileUpdateNotification|TestStartCompileUpdateCheckDoesNotBlockShutdown|TestStartCompileUpdateCheckSilentlyHandlesLockedDownNetwork|TestShouldCheckForUpdate|TestCheckForUpdatesWithNoCheckUpdateFlag|TestFindLatestPublishedReleaseTag|TestPrintCompilationSummaryWithFailedWorkflows|TestGetLastCheckFilePath|TestUpdateLastCheckTime' ./pkg/cligo test -v -tags integration -run 'TestUpdateCheckIntegration|TestUpdateCheckFlagHelp' ./pkg/climake agent-report-progressparallel_validation