Skip to content

[duplicate-code] Duplicate Code: Skill ref parsing duplicated between validation and resolution #51471

Description

@github-actions

Overview

The new skills-frontmatter feature now parses and classifies remote skill specs in two different code paths: validation and compile-time pinning. Both paths trim the raw spec, reject/skip expressions and local paths, split on @, treat empty refs specially, and short-circuit full SHA refs.

Key metrics/issues

  • Severity: Medium
  • Occurrences: 2
  • Analyzed files: 4 changed non-test Go files
  • Commit: ba0a9f958976c1d8d1baf3d2fc5a6aade16d01f2

Duplication details

Pattern: repeated parsing/classification of owner/repo[/path]@ref`` skill specs before either validating or pinning them.

Locations:

  • pkg/workflow/skills_frontmatter.go:58
  • pkg/workflow/skills_ref_resolution.go:47

Code sample:

trimmed := strings.TrimSpace(spec)
if trimmed == "" || strings.HasPrefix(trimmed, "${{") || isLocalSkillRef(trimmed) {
    return spec
}
repoPath, ref, hasAt := strings.Cut(trimmed, "@")
if !hasAt {
    return spec
}
if ref == "" { ... }
if gitutil.IsValidFullSHA(ref) { ... }

Impact analysis

  • Maintainability: The accepted shape of skill refs is now encoded in multiple places, so future syntax changes must be kept in sync manually.
  • Bug risk: Validation already owns checks for ambiguous SHA-like refs and unsafe characters, while resolution reimplements only part of the same decision tree. That split raises the chance of drift between “accepted” and “resolved” behavior.
  • Code bloat: The commit added a second parser/classifier instead of a shared helper or parsed struct.

Refactoring recommendations

  1. Extract a single skill-ref parser/classifier.
    Extract common functionality to: pkg/workflow/skills_frontmatter.go or a new focused helper in pkg/workflow/
    Estimated effort: 2-3 hours
    Benefits: one source of truth for local-vs-remote classification, repo path extraction, ref normalization, and SHA detection.

  2. Return structured metadata instead of reparsing strings.
    Estimated effort: low to medium
    Benefits: validation can enforce rules while resolution consumes the same parsed result without redoing branching logic.

Implementation checklist

  • Review duplication findings
  • Design shared skill-ref parse result type
  • Update validation to use shared parser
  • Update compile-time resolution to use shared parser
  • Update tests for edge cases
  • Verify warning/error behavior remains stable

Analysis metadata

  • Detection method: Serena semantic code analysis
  • Analysis date: 2026-08-08T21:59:53Z
  • Commit: ba0a9f958976c1d8d1baf3d2fc5a6aade16d01f2

Assignee: @copilot

Generated by 🔍 Duplicate Code Detector · gpt54 · 63.5 AIC · ⊞ 12.9K ·

  • expires on Aug 10, 2026, 2:02 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions