Skip to content

Commit 21ef0c6

Browse files
authored
Fix error message compliance: pre-activation jobs, GitHub tools validation, evals config, spec, and package manifest files (#52179)
1 parent 2f8d913 commit 21ef0c6

5 files changed

Lines changed: 84 additions & 84 deletions

File tree

pkg/cli/add_package_manifest.go

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func (e packageRemoteNotFoundError) Unwrap() []error {
7979
func resolveRepositoryPackage(ctx context.Context, repoSpec *RepoSpec, host string) (*resolvedRepositoryPackage, error) {
8080
parts := strings.SplitN(repoSpec.RepoSlug, "/", 2)
8181
if len(parts) != 2 {
82-
return nil, fmt.Errorf("invalid repository slug: %s", repoSpec.RepoSlug)
82+
return nil, fmt.Errorf("repository slug %q is not in 'owner/repo' format. Example: owner/repo", repoSpec.RepoSlug)
8383
}
8484

8585
owner := parts[0]
@@ -153,7 +153,7 @@ func resolveRepositoryPackage(ctx context.Context, repoSpec *RepoSpec, host stri
153153
warnings = append(warnings, agentWarnings...)
154154

155155
if len(installationSources) == 0 && len(skillFiles) == 0 && len(agentFiles) == 0 {
156-
return nil, fmt.Errorf("repository %q does not contain any installable workflows, skills, or agents (either explicitly declared or auto-discovered)", repositoryPackageIdentifier(repoSpec.RepoSlug, packagePath))
156+
return nil, fmt.Errorf("repository %q does not contain any installable workflows, skills, or agents (either explicitly declared or auto-discovered). Add workflows under 'workflows/', skills under 'skills/', or agents under 'agents/', or declare them explicitly in aw.yml", repositoryPackageIdentifier(repoSpec.RepoSlug, packagePath))
157157
}
158158

159159
return &resolvedRepositoryPackage{
@@ -179,12 +179,12 @@ func loadRepositoryPackageManifestFile(ctx context.Context, owner, repo, package
179179
content, err := downloadPackageFileFromGitHubForHost(ctx, owner, repo, manifestPath, ref, host)
180180
if err != nil {
181181
if !isRepositoryFileNotFound(err) {
182-
return "", nil, fmt.Errorf("failed to read manifest %q from %s/%s@%s: %w", manifestPath, owner, repo, ref, err)
182+
return "", nil, fmt.Errorf("failed to read manifest %q from %s/%s@%s (check the repository, ref, and network connectivity): %w", manifestPath, owner, repo, ref, err)
183183
}
184184
if packagePath != "" {
185-
return "", nil, fmt.Errorf("%w: repository %q is not a valid Agentic Workflow package: no aw.yml manifest found in %q; add %s or use an explicit workflow path", errRepositoryPackageManifestNotFound, packageID, packagePath, manifestPath)
185+
return "", nil, fmt.Errorf("%w: repository %q is not a valid Agentic Workflow package: no aw.yml manifest found in %q. Add %s or use an explicit workflow path", errRepositoryPackageManifestNotFound, packageID, packagePath, manifestPath)
186186
}
187-
return "", nil, fmt.Errorf("%w: repository %q is not a valid Agentic Workflow package: no aw.yml manifest found at the repository root; add aw.yml or use an explicit workflow path", errRepositoryPackageManifestNotFound, repoSlug)
187+
return "", nil, fmt.Errorf("%w: repository %q is not a valid Agentic Workflow package: no aw.yml manifest found at the repository root. Add aw.yml or use an explicit workflow path", errRepositoryPackageManifestNotFound, repoSlug)
188188
}
189189

190190
return manifestPath, content, nil
@@ -207,19 +207,19 @@ type repositoryPackageManifest struct {
207207
func parseRepositoryPackageManifest(manifestPath string, content []byte) (*repositoryPackageManifest, []string, error) {
208208
var raw any
209209
if err := yaml.Unmarshal(content, &raw); err != nil {
210-
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: %s", manifestPath, parser.FormatYAMLError(err, 1, string(content)))
210+
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: %s. Ensure the manifest is valid YAML. Example:\nname: My Package", manifestPath, parser.FormatYAMLError(err, 1, string(content)))
211211
}
212212

213213
root, ok := raw.(map[string]any)
214214
if !ok {
215-
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: top-level document must be a mapping", manifestPath)
215+
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: top-level document must be a mapping, not a list or scalar. Example:\nname: My Package", manifestPath)
216216
}
217217

218218
// Validate name before schema validation to provide a clear error message for
219219
// the most common manifest authoring error (missing or empty name).
220220
name, ok := stringValue(root["name"])
221221
if !ok || strings.TrimSpace(name) == "" {
222-
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: name must be a non-empty string", manifestPath)
222+
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: name must be a non-empty string. Example:\nname: My Package", manifestPath)
223223
}
224224

225225
if err := parser.ValidateRepositoryPackageManifestWithSchemaAndLocation(root, manifestPath); err != nil {
@@ -240,15 +240,15 @@ func parseRepositoryPackageManifest(manifestPath string, content []byte) (*repos
240240
if minVersion, ok := stringValue(root["min-version"]); ok {
241241
manifest.MinVersion = strings.TrimSpace(minVersion)
242242
if !isSupportedManifestMinVersion(manifest.MinVersion) {
243-
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version must use vMAJOR.minor.patch, got %q", manifestPath, minVersion)
243+
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version must use vMAJOR.minor.patch, got %q. Example:\nmin-version: v1.2.3", manifestPath, minVersion)
244244
}
245245
currentVersion := GetVersion()
246246
if !semverutil.IsValid(currentVersion) {
247-
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version validation requires a semantic-versioned compiler, but the current compiler version %q is not a valid semantic version (this indicates a build issue)", manifestPath, currentVersion)
247+
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version validation requires a semantic-versioned compiler, but the current compiler version %q is not a valid semantic version. This indicates a build issue; rebuild gh-aw with a proper version tag. Example: v1.2.3", manifestPath, currentVersion)
248248
}
249249
currentVersion = semverutil.NormalizeGitDescribeSemver(currentVersion)
250250
if semverutil.Compare(currentVersion, manifest.MinVersion) < 0 {
251-
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version %q requires gh-aw %s or newer (current: %s)", manifestPath, manifest.MinVersion, manifest.MinVersion, currentVersion)
251+
return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version %q requires gh-aw %s or newer (current: %s). Upgrade gh-aw, or lower min-version in aw.yml to a version at or below the current one. Example:\nmin-version: %s", manifestPath, manifest.MinVersion, manifest.MinVersion, currentVersion, currentVersion)
252252
}
253253
}
254254

@@ -597,7 +597,7 @@ func resolvePackageSkillFiles(ctx context.Context, owner, repo, packagePath, ref
597597
warnings = append(warnings, fmt.Sprintf("Skill directory %q is missing required %s marker file", skillDir, packageSkillMarkerFile))
598598
continue
599599
}
600-
return nil, nil, fmt.Errorf("failed to validate skill marker %q: %w", markerPath, err)
600+
return nil, nil, fmt.Errorf("failed to validate skill marker %q (check the repository, ref, and network connectivity): %w", markerPath, err)
601601
}
602602
}
603603
skillName := filepath.Base(skillDir)
@@ -609,7 +609,7 @@ func resolvePackageSkillFiles(ctx context.Context, owner, repo, packagePath, ref
609609
warnings = append(warnings, fmt.Sprintf("Skill directory %q not found in package, skipping", skillDir))
610610
continue
611611
}
612-
return nil, nil, fmt.Errorf("failed to list files in skill directory %q: %w", skillDir, err)
612+
return nil, nil, fmt.Errorf("failed to list files in skill directory %q (check the repository, ref, and network connectivity): %w", skillDir, err)
613613
}
614614
for _, file := range files {
615615
skillFiles = append(skillFiles, resolvedPackageSkillFile{
@@ -641,7 +641,7 @@ func resolvePackageAgentFiles(ctx context.Context, owner, repo, packagePath, ref
641641
if isRepositoryFileNotFound(err) {
642642
continue
643643
}
644-
return nil, nil, fmt.Errorf("failed to scan agents directory %q: %w", agentsDir, err)
644+
return nil, nil, fmt.Errorf("failed to scan agents directory %q (check the repository, ref, and network connectivity): %w", agentsDir, err)
645645
}
646646
for _, f := range files {
647647
if strings.HasSuffix(strings.ToLower(f), ".md") {
@@ -663,7 +663,7 @@ func scanPackageSkillDirs(ctx context.Context, owner, repo, packagePath, ref, ho
663663
if isRepositoryFileNotFound(err) {
664664
continue
665665
}
666-
return nil, fmt.Errorf("failed to scan skills directory %q: %w", skillsDir, err)
666+
return nil, fmt.Errorf("failed to scan skills directory %q (check the repository, ref, and network connectivity): %w", skillsDir, err)
667667
}
668668
for _, subdir := range subdirs {
669669
markerPath := joinRepositoryPackagePath(subdir, packageSkillMarkerFile)
@@ -686,7 +686,7 @@ func scanRepositoryPackageInstallablePaths(ctx context.Context, owner, repo, pac
686686
if isRepositoryFileNotFound(err) {
687687
continue
688688
}
689-
return nil, fmt.Errorf("failed to scan %q in %s/%s@%s: %w", sourcePath, owner, repo, ref, err)
689+
return nil, fmt.Errorf("failed to scan %q in %s/%s@%s (check the repository, ref, and network connectivity): %w", sourcePath, owner, repo, ref, err)
690690
}
691691

692692
for _, file := range files {
@@ -719,9 +719,9 @@ func resolveRepositoryPackageDocsPath(ctx context.Context, owner, repo, packageP
719719
if _, err := downloadPackageFileFromGitHubForHost(ctx, owner, repo, readmePath, ref, host); err == nil {
720720
return readmePath, nil
721721
} else if isRepositoryFileNotFound(err) {
722-
return "", fmt.Errorf("repository %q is not a valid Agentic Workflow package: missing required README.md at %q", packageID, readmePath)
722+
return "", fmt.Errorf("repository %q is not a valid Agentic Workflow package: missing required README.md at %q. Add a README.md describing the package. Example:\n# My Package\n\nDescribe what this package does.", packageID, readmePath)
723723
} else {
724-
return "", fmt.Errorf("failed to read package README %q from %s/%s@%s: %w", readmePath, owner, repo, ref, err)
724+
return "", fmt.Errorf("failed to read package README %q from %s/%s@%s (check the repository, ref, and network connectivity): %w", readmePath, owner, repo, ref, err)
725725
}
726726
}
727727

@@ -771,7 +771,7 @@ func validateManifestInstallableWorkflowPrivacy(manifestPath string, installatio
771771

772772
privateValue, hasPrivate := ExtractWorkflowPrivateSetting(string(content))
773773
if hasPrivate && privateValue {
774-
return fmt.Errorf("invalid Agentic Workflow manifest %q: workflow %q sets private: true and cannot be included because private workflows cannot be added", manifestPath, installationSource)
774+
return fmt.Errorf("invalid Agentic Workflow manifest %q: workflow %q sets private: true and cannot be included because private workflows cannot be added. Remove 'private: true' from the workflow frontmatter or exclude it from the manifest. Example:\n---\nprivate: false\n---", manifestPath, installationSource)
775775
}
776776
}
777777

@@ -829,7 +829,7 @@ func parseRepositoryPackageSpec(spec string) (*RepoSpec, bool, error) {
829829
if cleanedPath == "." {
830830
packagePath = ""
831831
} else if cleanedPath == ".." || strings.HasPrefix(cleanedPath, "../") {
832-
return nil, true, fmt.Errorf("invalid repository package path %q", packagePath)
832+
return nil, true, fmt.Errorf("invalid repository package path %q: path traversal outside the repository is not allowed. Use a path relative to the repository root. Example: packages/my-package", packagePath)
833833
} else {
834834
packagePath = cleanedPath
835835
}
@@ -883,7 +883,7 @@ func validateUniqueManifestWorkflowFilenames(paths []string, manifestPath string
883883
continue
884884
}
885885
if previous, exists := seen[key]; exists {
886-
return fmt.Errorf("invalid Agentic Workflow manifest %q: duplicate workflow filename %q in files entries %q and %q (filenames must be unique across a package)", manifestPath, filenameWithoutExt, previous, installPath)
886+
return fmt.Errorf("invalid Agentic Workflow manifest %q: duplicate workflow filename %q in files entries %q and %q. Filenames must be unique across a package; rename one of the workflow files. Example:\nfiles:\n - workflows/%s.md\n - workflows/%s-2.md", manifestPath, filenameWithoutExt, previous, installPath, filenameWithoutExt, filenameWithoutExt)
887887
}
888888
seen[key] = installPath
889889
}
@@ -952,7 +952,7 @@ func resolveRepositoryPackageDefaultBranch(ctx context.Context, repoSlug, host s
952952
if targetHost == "" {
953953
targetHost = "the configured host"
954954
}
955-
return "", fmt.Errorf("repository %s on %s returned an empty default branch; ensure the repository exists and is accessible", repoSlug, targetHost)
955+
return "", fmt.Errorf("repository %s on %s returned an empty default branch. Ensure the repository exists and is accessible", repoSlug, targetHost)
956956
}
957957
return branch, nil
958958
}

pkg/cli/spec.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func parseRepoSpec(repoSpec string) (*RepoSpec, error) {
127127
repoURL, err := url.Parse(repo)
128128
if err != nil {
129129
specLog.Printf("Failed to parse GitHub URL: %v", err)
130-
return nil, fmt.Errorf("invalid GitHub URL: %w", err)
130+
return nil, fmt.Errorf("could not parse GitHub URL %q (use a URL like https://github.com/owner/repo): %w", repo, err)
131131
}
132132

133133
// Extract owner/repo from path
@@ -145,7 +145,7 @@ func parseRepoSpec(repoSpec string) (*RepoSpec, error) {
145145
currentRepo, err := GetCurrentRepoSlug()
146146
if err != nil {
147147
specLog.Printf("Failed to get current repo: %v", err)
148-
return nil, fmt.Errorf("failed to get current repository info: %w", err)
148+
return nil, fmt.Errorf("failed to get current repository info (run this command from inside a git repository with a GitHub remote, or specify 'owner/repo' explicitly): %w", err)
149149
}
150150
repo = currentRepo
151151
specLog.Printf("Resolved current repo: %s", repo)
@@ -181,15 +181,15 @@ func parseGitHubURL(spec string) (*WorkflowSpec, error) {
181181
parsedURL, err := url.Parse(spec)
182182
if err != nil {
183183
specLog.Printf("Failed to parse URL: %v", err)
184-
return nil, fmt.Errorf("invalid URL: %w", err)
184+
return nil, fmt.Errorf("could not parse URL %q (use a URL like https://github.com/owner/repo/blob/main/workflows/workflow.md): %w", spec, err)
185185
}
186186

187187
if parsedURL.Host == "" {
188-
return nil, fmt.Errorf("URL must include a host: %s", spec)
188+
return nil, fmt.Errorf("URL %q is missing a host. Use a full URL. Example: https://github.com/owner/repo/blob/main/workflows/workflow.md", spec)
189189
}
190190

191191
if !isGitHubHost(parsedURL.Host) {
192-
return nil, fmt.Errorf("URL must be from github.com or a GitHub Enterprise host (*.ghe.com), got %q", parsedURL.Host)
192+
return nil, fmt.Errorf("URL host %q is not supported. Expected github.com or a GitHub Enterprise host (*.ghe.com). Example: https://github.com/owner/repo/blob/main/workflows/workflow.md", parsedURL.Host)
193193
}
194194

195195
owner, repo, ref, filePath, err := parser.ParseRepoFileURL(spec)
@@ -202,12 +202,12 @@ func parseGitHubURL(spec string) (*WorkflowSpec, error) {
202202

203203
// Ensure the file path ends with .md
204204
if !strings.HasSuffix(filePath, ".md") {
205-
return nil, errors.New("GitHub URL must point to a .md file")
205+
return nil, errors.New("GitHub URL must point to a .md file. Example: https://github.com/owner/repo/blob/main/workflows/workflow.md")
206206
}
207207

208208
// Validate owner and repo
209209
if !parser.IsValidGitHubIdentifier(owner) || !parser.IsValidGitHubRepositoryName(repo) {
210-
return nil, fmt.Errorf("invalid GitHub URL: '%s/%s' does not look like a valid GitHub repository", owner, repo)
210+
return nil, fmt.Errorf("GitHub URL contains '%s/%s', which does not look like a valid GitHub repository. Expected owner and repository names with only letters, numbers, hyphens, and underscores", owner, repo)
211211
}
212212

213213
// For raw.githubusercontent.com content, the API host is github.com.
@@ -294,10 +294,10 @@ func parseWorkflowSpec(spec string) (*WorkflowSpec, error) {
294294
// Non-GitHub HTTP(S) URL: return a generic URL spec whose content will be
295295
// fetched at resolution time and dispatched on Content-Type.
296296
if urlErr != nil {
297-
return nil, fmt.Errorf("invalid URL %q: %w", spec, urlErr)
297+
return nil, fmt.Errorf("could not parse URL %q (use a fully qualified http(s) URL): %w", spec, urlErr)
298298
}
299299
if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
300-
return nil, fmt.Errorf("unsupported URL scheme %q: only http and https are supported", parsedURL.Scheme)
300+
return nil, fmt.Errorf("URL scheme %q is not supported. Only http and https are supported. Example: https://example.com/workflow.md", parsedURL.Scheme)
301301
}
302302
specLog.Printf("Detected generic import URL: %s", spec)
303303
return &WorkflowSpec{
@@ -341,7 +341,7 @@ func parseWorkflowSpec(spec string) (*WorkflowSpec, error) {
341341

342342
// Must have at least 3 parts: owner/repo/workflow-path
343343
if len(slashParts) < 3 {
344-
return nil, errors.New("workflow specification must be in format 'owner/repo/workflow-name[@version]'")
344+
return nil, errors.New("workflow specification format is not recognized. Expected 'owner/repo/workflow-name[@version]'. Example: github/gh-aw/ci-doctor")
345345
}
346346

347347
owner := slashParts[0]
@@ -367,12 +367,12 @@ func parseWorkflowSpec(spec string) (*WorkflowSpec, error) {
367367

368368
// Validate owner and repo parts are not empty
369369
if owner == "" || repo == "" {
370-
return nil, errors.New("invalid workflow specification: owner and repo cannot be empty")
370+
return nil, errors.New("workflow specification is missing owner or repo. Expected 'owner/repo/workflow-name[@version]'. Example: github/gh-aw/ci-doctor")
371371
}
372372

373373
// Basic validation that owner and repo look like GitHub identifiers
374374
if !parser.IsValidGitHubIdentifier(owner) || !parser.IsValidGitHubRepositoryName(repo) {
375-
return nil, fmt.Errorf("invalid workflow specification: '%s/%s' does not look like a valid GitHub repository", owner, repo)
375+
return nil, fmt.Errorf("workflow specification contains '%s/%s', which does not look like a valid GitHub repository. Expected owner and repository names with only letters, numbers, hyphens, and underscores", owner, repo)
376376
}
377377

378378
repoSlug := fmt.Sprintf("%s/%s", owner, repo)
@@ -407,7 +407,7 @@ func parseWorkflowSpec(spec string) (*WorkflowSpec, error) {
407407
// Four or more parts: owner/repo/workflows/workflow-name or owner/repo/path/to/workflow-name
408408
// Require .md extension to be explicit
409409
if !strings.HasSuffix(workflowPath, ".md") {
410-
return nil, fmt.Errorf("workflow specification with path must end with '.md' extension: %s", workflowPath)
410+
return nil, fmt.Errorf("workflow specification path %q must end with '.md' extension. Example: owner/repo/workflows/ci-doctor.md", workflowPath)
411411
}
412412
}
413413

@@ -428,7 +428,7 @@ func parseLocalWorkflowSpec(spec string) (*WorkflowSpec, error) {
428428
// Validate that it's a .md file
429429
if !strings.HasSuffix(spec, ".md") {
430430
specLog.Printf("Invalid extension for local workflow: %s", spec)
431-
return nil, fmt.Errorf("local workflow specification must end with '.md' extension: %s", spec)
431+
return nil, fmt.Errorf("local workflow specification %q must end with '.md' extension. Example: ./workflows/ci-doctor.md", spec)
432432
}
433433

434434
specLog.Printf("Parsed local workflow: path=%s", spec)

0 commit comments

Comments
 (0)