Skip to content

[file-diet] Refactor pkg/parser/remote_fetch.go (1553 lines) into focused modules #43792

Description

@github-actions

Overview

The file pkg/parser/remote_fetch.go has grown to 1553 lines, making it difficult to maintain, navigate, and test. This task involves splitting it into smaller, focused files grouped by functional domain — consistent with the repository's code organisation patterns.

Current State

  • File: pkg/parser/remote_fetch.go
  • Size: 1553 lines
  • Test Coverage: pkg/parser/remote_fetch_test.go — 84 lines (test-to-source ratio ≈ 0.05; very low)
  • Complexity: Five distinct functional domains are co-located in one file with significant duplication in the fallback-retry pattern (authenticated API → git → public API) repeated across 4+ listing functions
Full File Analysis

Functional Domain Breakdown

Domain Approx. Lines Key Functions
Path resolution (local/builtin) ~180 ResolveIncludePath, resolveBuiltinIncludePath, findGitHubFolder, computeIncludeResolveAndSecurityBases, resolveAndValidateLocalIncludePath
WorkflowSpec parsing & caching ~160 IsWorkflowSpec, isWorkflowSpec, downloadIncludeFromWorkflowSpec, parseWorkflowSpecParts, resolveWorkflowSpecSHAForCache, writeDownloadedIncludeToTempFile
Ref-to-SHA resolution ~155 resolveRefToSHA, resolveRefToSHAViaGit, resolveRefToSHAViaPublicAPI, buildCommitLookupAPIPath
File download (single file) ~430 DownloadFileFromGitHub, DownloadFileFromGitHubForHost, downloadFileFromGitHub, downloadFileFromGitHubWithDepth, downloadFileViaGit, downloadFileViaRawURL, downloadFileViaGitClone, checkRemoteSymlink, resolveRemoteSymlinks, resolveRemoteSymlinkComponent, resolveAndValidateRemoteSymlinkBase, retryDownloadViaResolvedSymlink, fetchRemoteFileContent, buildContentsAPIPath, createRESTClientForHost, downloadFileViaPublicAPI
Directory listing ~490 ListWorkflowFiles, ListWorkflowFilesForHost, ListDirAllFilesForHost, ListDirAllFilesRecursivelyForHost, ListDirSubdirsForHost, + all private list* variants, fetchPublicGitHubContentsAPI, getOrCreateListRepoClone

Duplication Hotspot

The three-tier fallback pattern (authenticated REST → git CLI → public unauthenticated API) is copy-pasted across every listing operation and the single-file download, adding ~200 lines of near-identical error-handling code.

Shared State

publicAPIClient and gitListCloneCache package-level vars are only used by a subset of functions; splitting the file makes ownership clearer.

Refactoring Strategy

Proposed File Splits

  1. remote_resolve_path.go

    • Functions: ResolveIncludePath, resolveBuiltinIncludePath, findGitHubFolder, computeIncludeResolveAndSecurityBases, resolveAndValidateLocalIncludePath, isUnderWorkflowsDirectory, isCustomAgentFile, isRepositoryImport
    • Responsibility: Local and builtin include-path resolution, security validation
    • Estimated LOC: ~200
  2. remote_workflow_spec.go

    • Functions: IsWorkflowSpec, isWorkflowSpec, downloadIncludeFromWorkflowSpec, parseWorkflowSpecParts, resolveWorkflowSpecSHAForCache, writeDownloadedIncludeToTempFile
    • Responsibility: WorkflowSpec format parsing, caching, and dispatch
    • Estimated LOC: ~150
  3. remote_resolve_sha.go

    • Functions: resolveRefToSHA, resolveRefToSHAViaGit, resolveRefToSHAViaPublicAPI, buildCommitLookupAPIPath, ResolveRefToSHAForHost
    • Responsibility: Resolving git refs (branch/tag/SHA) to full SHAs with fallback chain
    • Estimated LOC: ~165
  4. remote_download_file.go

    • Functions: DownloadFileFromGitHub, DownloadFileFromGitHubForHost, downloadFileFromGitHub, downloadFileFromGitHubWithDepth, downloadFileViaGit, downloadFileViaRawURL, downloadFileViaGitClone, retryDownloadViaResolvedSymlink, checkRemoteSymlink, resolveRemoteSymlinks, resolveRemoteSymlinkComponent, resolveAndValidateRemoteSymlinkBase, fetchRemoteFileContent, createRESTClientForHost, buildContentsAPIPath, downloadFileViaPublicAPI
    • Responsibility: Downloading a single file from GitHub with auth/git/public fallback chain
    • Estimated LOC: ~480
  5. remote_list_files.go

    • Functions: ListWorkflowFiles, ListWorkflowFilesForHost, ListDirAllFilesForHost, ListDirAllFilesRecursivelyForHost, ListDirSubdirsForHost + all private list* variants, getOrCreateListRepoClone, fetchPublicGitHubContentsAPI
    • Responsibility: Directory-listing operations (workflow files, all files, recursive, subdirs)
    • Estimated LOC: ~490

Shared Utilities

Move createRESTClientForHost, buildContentsAPIPath, fetchRemoteFileContent, fetchPublicGitHubContentsAPI, and the publicAPIClient var to remote_download_file.go or a new remote_client.go (~80 lines) so both download and listing code can share them without circular deps.

Interface Abstractions

Consider extracting the three-tier fallback pattern into a helper: withAuthFallback(apiCall, gitFallback, publicFallback) — this would eliminate ~200 lines of repetitive error-handling code across 4 listing functions.

Test Coverage Plan

Current test coverage is very low (84 test lines vs 1553 source lines). Add tests for each new file:

  1. remote_resolve_path_test.go

    • Test cases: local path within .github, path escaping security boundary, builtin path prefix, .agents/ prefix handling
    • Target coverage: ≥80%
  2. remote_workflow_spec_test.go

    • Test cases: IsWorkflowSpec edge cases (URL-like, local dotfile, short paths), parseWorkflowSpecParts ref defaulting, cache hit/miss
    • Target coverage: ≥80%
  3. remote_resolve_sha_test.go

    • Test cases: already-a-SHA passthrough, API success, auth error → git fallback → public API fallback
    • Target coverage: ≥80%
  4. remote_download_file_test.go

    • Test cases: base64 decode, symlink depth guard, auth fallback chain, raw URL success path
    • Target coverage: ≥80%
  5. remote_list_files_test.go

    • Test cases: workflow file filter (only .md direct children), recursive listing depth guard, auth fallback chain for each listing variant
    • Target coverage: ≥80%

Implementation Guidelines

  1. Preserve Behavior: Ensure all existing functionality works identically
  2. Maintain Exports: Keep public API unchanged (ResolveIncludePath, IsWorkflowSpec, DownloadFileFromGitHub, ListWorkflowFiles, etc.)
  3. Add Tests First: Write tests for each new file before moving functions
  4. Incremental Changes: Split one domain at a time; keep the build green between each split
  5. Run Tests Frequently: Verify make test-unit passes after each split
  6. Update Imports: No import path changes needed — same package parser
  7. Remove remote_fetch.go: Delete the original file only after all functions have been moved

Acceptance Criteria

  • pkg/parser/remote_fetch.go is split into 4–5 focused files
  • Each new file is under 500 lines
  • All tests pass (make test-unit)
  • Test coverage is ≥80% for new files
  • No breaking changes to public API
  • Code passes linting (make lint)
  • Build succeeds (make build)
Additional Context
  • Repository Guidelines: Follow patterns in .github/agents/developer.instructions.agent.md and AGENTS.md
  • Code Organisation: One functional domain per file; prefer many small files
  • Build Tag: remote_fetch.go has //go:build !js && !wasm — preserve this tag in all split files
  • Testing: Match existing test patterns in pkg/parser/*_test.go

Priority: Medium
Effort: Medium (mechanical move + test-writing; no logic changes required)
Expected Impact: Improved maintainability, easier per-domain testing, reduced cognitive load when navigating parser code

Generated by 🧹 Daily File Diet · 61.3 AIC · ⌖ 13.5 AIC · ⊞ 6.9K ·

  • expires on Jul 8, 2026, 5:59 AM 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