Skip to content

Add scope enum validation to cache-memory parser#19406

Merged
pelikhan merged 4 commits intomainfrom
copilot/add-cache-memory-scope-validation
Mar 3, 2026
Merged

Add scope enum validation to cache-memory parser#19406
pelikhan merged 4 commits intomainfrom
copilot/add-cache-memory-scope-validation

Conversation

Copy link
Contributor

Copilot AI commented Mar 3, 2026

  • Analyze code: schema already has enum, only parser validation missing
  • Add scope validation in parseCacheMemoryEntry() in pkg/workflow/cache.go using slices.Contains
  • Add unit tests in pkg/workflow/cache_scope_validation_test.go using testify assertions
  • Define validCacheMemoryScopes at package scope as single source of truth for validation and error messages
  • make fmt lint build passes with 0 issues
Original prompt

This section details on the original issue you should resolve

<issue_title>[plan] Add cache-memory scope enum validation to schema and parser</issue_title>
<issue_description>## Objective

Add enum validation for the scope field in parseCacheMemoryEntry() so invalid values are rejected with a clear error rather than silently treated as "workflow".

Context

From discussion #19383 (MEDIUM-1): The cache-memory scope field accepts only "workflow" and "repo", but:

  1. The JSON schema (main_workflow_schema.json) doesn't define these as an enum — any string passes schema validation
  2. The parser (parseCacheMemoryEntry()) sets entry.Scope from any string
  3. Downstream logic at cache.go:406 only checks if scope == "repo", so any other value (e.g., "organization", "global") silently behaves like "workflow"

Approach

  1. In the JSON schema (pkg/parser/schemas/main_workflow_schema.json): Add "enum": ["workflow", "repo"] to the cache-memory[*].scope property definition

  2. In the parser (pkg/workflow/ — wherever parseCacheMemoryEntry() is defined): After extracting entry.Scope, validate it:

    validScopes := []string{"workflow", "repo"}
    if !contains(validScopes, entry.Scope) {
        return nil, fmt.Errorf("invalid cache-memory scope %q: must be one of [workflow, repo]", entry.Scope)
    }
  3. Run make build after schema changes (schemas are embedded via //go:embed)

Files to Modify

  • pkg/parser/schemas/main_workflow_schema.json — add enum to cache-memory[].scope
  • Parser file containing parseCacheMemoryEntry() — add scope validation

Acceptance Criteria

  • Schema defines scope as enum: [workflow, repo]
  • Parser returns a clear error message for invalid scope values
  • "workflow" and "repo" continue to work correctly
  • Unit tests cover valid and invalid scope values
  • make build run after schema changes
  • Run make agent-finish before committing

Generated by Plan Command for issue #discussion #19383 ·

  • expires on Mar 5, 2026, 10:49 AM UTC

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add enum validation for cache-memory scope in schema Add scope enum validation to cache-memory parser Mar 3, 2026
@pelikhan pelikhan marked this pull request as ready for review March 3, 2026 11:21
Copilot AI review requested due to automatic review settings March 3, 2026 11:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enforces the existing cache-memory.scope enum at parse time so invalid scope values are rejected instead of silently behaving like the default "workflow" scope.

Changes:

  • Add explicit scope validation in parseCacheMemoryEntry() (only "workflow" and "repo" are accepted).
  • Add unit tests covering valid scopes (object/array forms), invalid scopes, and defaulting to "workflow" when omitted.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pkg/workflow/cache.go Validates cache-memory.scope against the allowed enum and returns a descriptive error on invalid values.
pkg/workflow/cache_scope_validation_test.go Adds tests for scope validation across object/array tool syntaxes and for default scope behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +116 to +119
// Validate scope value
if !slices.Contains([]string{"workflow", "repo"}, entry.Scope) {
return entry, fmt.Errorf("invalid cache-memory scope %q: must be one of [workflow, repo]", entry.Scope)
}
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list of valid scopes is duplicated (once in the slices.Contains call and again in the error string). This can drift if a new scope is added. Define a validScopes slice once (optionally at package scope) and use it both for validation and for formatting the error message (e.g., %v), so there’s a single source of truth.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@pelikhan
Copy link
Contributor

pelikhan commented Mar 3, 2026

@copilot apply comments

… formatting

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 3, 2026

@copilot apply comments

Applied both comments in 0d62b4f:

  • validCacheMemoryScopes is now defined once at package scope and used for both slices.Contains validation and the error message %v format — no more duplication.
  • Test case already renamed to "invalid whitespace scope" in the prior commit.

@pelikhan pelikhan merged commit 93a5b62 into main Mar 3, 2026
77 checks passed
@pelikhan pelikhan deleted the copilot/add-cache-memory-scope-validation branch March 3, 2026 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[plan] Add cache-memory scope enum validation to schema and parser

3 participants