Skip to content

[refactor] pkg/parser: wasm build-tag copies have drifted, plus 3 duplicate-function clustersΒ #53888

Description

@github-actions

πŸ”§ Semantic Function Clustering Analysis

Scope: pkg/parser (53 non-test files) and pkg/modelsdev (1 file) β€” the precomputed package slice for this run.

Executive Summary

Cataloged ~580 non-test functions across 54 files. Overall organization is good: the schedule_*, schema_*, include_*, and import_* file clusters each have a clear single purpose, and no meaningful "function in the wrong file" outliers were found. pkg/modelsdev/catalog.go (2 functions) is clean.

Four concrete duplication findings survived verification. One of them (#1) is not just cosmetic β€” the two copies have actually drifted apart in behavior.

# Finding Severity
1 Wasm/non-wasm copies of remote path resolution have drifted High β€” behavioral divergence
2 splitImportPathAndSection / splitIncludePathAndSection byte-identical Medium
3 extractFrontmatterForTopologicalSort duplicates extractFrontmatterForImport Medium
4 Four near-identical "list with API→git→public-API fallback" chains Medium

1. Wasm build-tag copies have drifted (High)

pkg/parser/remote_fetch_wasm.go ((go/redacted):build js || wasm) hand-duplicates four functions that also exist in the (go/redacted):build !js && !wasm files remote_resolve_path.go and remote_workflow_spec.go. These are not thin platform shims β€” they are copy-pasted logic, and the copies no longer agree.

Confirmed divergence in isRepositoryImport:

// remote_resolve_path.go:92 (native) β€” rejects only known data/workflow extensions
for _, ext := range []string{".md", ".yaml", ".yml", ".json"} {
    if strings.HasSuffix(strings.ToLower(repo), ext) {
        return false
    }
}

// remote_fetch_wasm.go:52 (wasm) β€” rejects ANY dot in the repo name
if strings.Contains(repo, ".") {
    return false
}

The wasm predicate is strictly more restrictive. An import spec like githubnext/gh-aw.dev resolves as a repository import on native builds and is silently rejected on wasm. Any repo whose name legitimately contains a dot behaves differently depending on build target.

Secondary drift β€” constants vs. hardcoded literals:

remote_resolve_path.go uses constants.WorkflowsDirSlash, constants.AgentsDir, and constants.GithubDir. The wasm copy hardcodes ".github/workflows/", ".github/agents/", and ".github/". Changing a constant updates only one of the two code paths.

IsWorkflowSpec is duplicated near-verbatim (~50 lines) at remote_workflow_spec.go:16 and remote_fetch_wasm.go:129, including the identical "Preserve legacy behavior expected by parser tests" comment. The wasm file also adds a redundant one-line alias isWorkflowSpec (remote_fetch_wasm.go:165) that exists only to call IsWorkflowSpec.

The repo already has the right pattern for this. GetGitHubToken is split across github.go (native) and github_wasm.go (wasm), but the shared env-var logic lives in the build-tag-free github_token_env.go and both call into it. github_wasm.go is 17 lines because of it.

Recommendation: apply the same treatment. Move the platform-independent predicates β€” isUnderWorkflowsDirectory, isCustomAgentFile, isRepositoryImport, IsWorkflowSpec β€” plus the pure path arithmetic in findGitHubFolder and computeIncludeResolveAndSecurityBases into a build-tag-free file (e.g. remote_path_predicates.go). Only ResolveIncludePath's final filesystem probe genuinely differs (os.Stat vs VirtualFileExists). Decide deliberately which isRepositoryImport behavior is correct rather than leaving both. Delete the isWorkflowSpec alias.

Impact: removes ~120 duplicated lines and eliminates a class of silent native/wasm behavior differences.


2. Byte-identical path/section splitters (Medium)

Three functions in the same package implement the same path#section split:

Function Location Callers
splitImportPathAndSection pkg/parser/import_bfs.go:143 import_bfs.go:131, import_bfs.go:402
splitIncludePathAndSection pkg/parser/include_processor.go:163 include_processor.go:135
stripImportSection pkg/parser/import_topological.go:93 import_topological.go:77

The first two are identical apart from the parameter name:

func splitImportPathAndSection(importPath string) (string, string) {
	if strings.Contains(importPath, "#") {
		parts := strings.SplitN(importPath, "#", 2)
		return parts[0], parts[1]
	}
	return importPath, ""
}

stripImportSection is the same logic discarding the second return value.

Recommendation: keep one exported-within-package helper (splitPathAndSection); replace stripImportSection's single call site with path, _ := splitPathAndSection(...). All five call sites are in pkg/parser, so this is a contained change.


3. Duplicate frontmatter extraction for builtin paths (Medium)

// pkg/parser/import_bfs.go:367
func extractFrontmatterForImport(fullPath string, content []byte) (*FrontmatterResult, error) {
	if strings.HasPrefix(fullPath, BuiltinPathPrefix) {
		return ExtractFrontmatterFromBuiltinFile(fullPath, content)
	}
	return ExtractFrontmatterFromContent(string(content))
}

// pkg/parser/import_topological.go:101
func extractFrontmatterForTopologicalSort(fullPath string, content []byte) (map[string]any, error) {
	var (result *FrontmatterResult; err error)
	if strings.HasPrefix(fullPath, BuiltinPathPrefix) {
		result, err = ExtractFrontmatterFromBuiltinFile(fullPath, content)
	} else {
		result, err = ExtractFrontmatterFromContent(string(content))
	}
	if err != nil { return nil, err }
	return result.Frontmatter, nil
}

Identical branch logic; the second only unwraps .Frontmatter. Both are unexported in the same package.

Recommendation: delete extractFrontmatterForTopologicalSort and have its one caller (import_topological.go) call extractFrontmatterForImport and read .Frontmatter.


4. Four repeated list-with-fallback chains (Medium)

pkg/parser/remote_list_files.go β€” 12 functions, ~460 lines, one shape repeated four times

Each of four listing operations is implemented as the same three-function chain: authenticated Contents API β†’ git-clone fallback on auth error β†’ unauthenticated public API fallback (github.com only).

Operation Primary Git fallback Public-API fallback
Workflow .md files listWorkflowFilesForHost:179 listWorkflowFilesViaGitForHost:231 listWorkflowFilesViaPublicAPI:272
All dir files listDirAllFilesForHost:305 listDirAllFilesViaGitForHost:350 listDirAllFilesViaPublicAPI:386
Subdirectories listDirSubdirsForHost:525 listDirSubdirsViaGitForHost:570 listDirSubdirsViaPublicAPI:606
Recursive files listDirAllFilesRecursivelyForHost:418 listDirAllFilesRecursivelyViaGitForHost:487 (intentionally none β€” rate limits)

The three complete chains differ only in:

  • the entry filter β€” Type == "file" && .md / Type == "file" / Type == "dir"
  • the git ls-tree flags β€” -r vs -d
  • log and error message wording

The anonymous response struct {Name, Path, Type string} with identical JSON tags is redeclared six times in this file (lines 189, 279, 314, 393, 534, 613). The auth-error-then-git-then-public-API control flow is reproduced verbatim three times (lines 198–217, 322–337, 542–557).

Note this file is also where the anonymous-struct redeclaration causes a real inconsistency: listDirAllFilesViaPublicAPI:393 declares only {Path, Type} while its five siblings declare {Name, Path, Type} β€” harmless today, but exactly the kind of drift copy-paste produces.

Recommendation: extract one generic helper parameterized by a predicate and the ls-tree mode, e.g.

type contentEntry struct {
	Name string `json:"name"`
	Path string `json:"path"`
	Type string `json:"type"`
}

func listRemoteEntries(ctx context.Context, owner, repo, ref, dirPath, host string,
	keep func(contentEntry) bool, gitMode lsTreeMode, label string) ([]string, error)

The four public entry points become thin wrappers. This is the largest single line-count reduction available in the slice (~250 lines), but it touches the auth-fallback path, so it should land with the existing remote_fetch_test.go fallback tests green.


Minor observation (not recommended for action)

findGitHubRepoRoot (include_expander.go:122) and findGitHubFolder (remote_resolve_path.go:130) both walk ancestors looking for .github, but return different things (parent-of-.github vs. the folder itself) and have different not-found behavior ("" vs. fall back to baseDir). Genuinely distinct contracts β€” flagged only so a future consolidation pass does not mistake them for duplicates.


Suggested order of work

Analysis metadata
  • Files in scope: 54 (53 in pkg/parser, 1 in pkg/modelsdev); _test.go excluded
  • Non-test functions cataloged: ~580
  • Clusters examined: schema_*, import_*, remote_*, schedule_*, include_* / inline_*, frontmatter_*, github*, virtual_fs*
  • Outliers (wrong-file functions) found: 0
  • Duplicate groups confirmed: 4
  • Method: symbol inventory per file, naming-pattern clustering, then side-by-side source comparison of every candidate pair. All four findings were verified by reading both implementations; none are inferred from names alone.

Generated by πŸ”§ Semantic Function Refactoring Β· sonnet46 Β· 222.8 AIC Β· βŒ– 21.3 AIC Β· ⊞ 10.1K Β· β—·

  • expires on Aug 20, 2026, 6:48 PM UTC-08:00

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions