Consolidate CLI not-found error classification - #52621
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Consolidates CLI not-found detection through errorutil, fixing title-case Not Found handling from issue #52611.
Changes:
- Adds a case-insensitive raw-output classifier.
- Replaces duplicated CLI predicates and temporary error wrappers.
- Adds regression coverage for classifier variants and remote experiments.
Show a summary per file
| File | Description |
|---|---|
pkg/errorutil/errors.go |
Adds shared raw-output classification. |
pkg/errorutil/errors_test.go |
Tests output variants. |
pkg/cli/workflow_run_metadata.go |
Removes temporary error wrapping. |
pkg/cli/setup_repository.go |
Centralizes repository not-found detection. |
pkg/cli/logs_download.go |
Classifies raw CLI output directly. |
pkg/cli/experiments_command.go |
Fixes case-insensitive experiment detection. |
pkg/cli/experiments_command_test.go |
Adds remote experiment regression coverage. |
pkg/cli/evals_branch.go |
Reuses the shared classifier. |
pkg/cli/checks_command.go |
Replaces inline matching. |
pkg/cli/branch_file_reader.go |
Removes the duplicate output helper. |
pkg/cli/add_package_manifest.go |
Removes the duplicate error helper. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Balanced
| func TestFetchRemoteExperimentDetailsClassifiesTitleCaseNotFound(t *testing.T) { | ||
| fakeBinDir := t.TempDir() | ||
| fakeGH := filepath.Join(fakeBinDir, "gh") | ||
| require.NoError(t, os.WriteFile(fakeGH, []byte("#!/bin/sh\necho 'HTTP 404: Not Found' >&2\nexit 1\n"), 0o755)) |
| // IsNotFoundOutput reports whether output represents an HTTP 404 / "not found" response. | ||
| // The check is case-insensitive and matches both the numeric literal "404" and | ||
| // the phrase "not found". | ||
| func IsNotFoundOutput(output string) bool { |
Triage: refactor / risk: lowScore: 37/100 (impact+urgency+quality) · Priority: medium · Action: Consolidates CLI not-found error classification into errorutil, fixes case-sensitivity gap. CI green. Automated PR triage — see full report issue for details.
|
PR Triage\n\n- Category: refactor\n- Risk: low\n- Priority: medium\n- Score: 48/100 (impact 25 + urgency 15 + quality 8)\n- Recommended action:
|
PR Triage
Automated triage by PR Triage Agent.
|
|
@copilot Please run the Failed checks:
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
PR TriageCategory: Score breakdown
Recommended action:
|
Several CLI paths duplicated
404/not founddetection instead of usingerrorutil. One case was case-sensitive and missed GitHub’s title-caseNot Foundoutput.Shared classification
errorutil.IsNotFoundOutput(string)for raw CLI output.CLI consolidation
errors.Newwrappers.Regression coverage
Not Found.