-
Notifications
You must be signed in to change notification settings - Fork 296
Description
Overview
The file pkg/cli/remote_workflow.go has grown to 1255 lines, combining five distinct functional domains into a single monolithic file. This makes the code harder to maintain, test, and understand. Splitting this file into smaller, focused modules will improve code organization and maintainability.
Current State
- File:
pkg/cli/remote_workflow.go - Size: 1255 lines
- Functions: 15 public and private functions
- Test Coverage: 1735 lines in
remote_workflow_test.gowith 60 test cases - Status: Well-tested but monolithic
Functional Analysis
The file contains five distinct, semi-independent domains:
-
Workflow Source Fetching (lines 36-157): Core public API for fetching workflows
FetchWorkflowFromSource()- Main entry pointfetchLocalWorkflow()- Local filesystem accessfetchRemoteWorkflow()- GitHub API integration- Responsibilities: Local/remote workflow resolution, commit SHA tracking
-
Include File Fetching (lines 158-577): Remote includes and frontmatter imports
FetchIncludeFromSource()- Include file fetchingfetchAndSaveRemoteFrontmatterImports()- Frontmatter import handlingfetchFrontmatterImportsRecursive()- Recursive import resolutionfetchAndSaveRemoteIncludes()-@includedirective processing- Responsibilities: Include resolution, recursive import handling, file I/O
-
Dispatch Workflow Processing (lines 628-1078): Dispatch workflow extraction and management
extractDispatchWorkflowNames()- Parse workflow names from YAMLfetchAndSaveRemoteDispatchWorkflows()- Download and track dispatch workflowsfetchAndSaveDispatchWorkflowsFromParsedFile()- Handle dispatch workflows from parsed files- Responsibilities: Workflow name extraction, dispatch workflow downloading, conflict detection
-
Resource File Handling (lines 852-1077): Resource file fetching and caching
extractResources()- Parse resource paths from workflowsfetchAndSaveRemoteResources()- Download and track resource files- Responsibilities: Resource extraction, remote resource downloading
-
Helper Utilities (lines 579-627): Shared utility functions
getParentDir()- Path manipulationreadSourceRepoFromFile()- Source metadata readingsourceRepoLabel()- Label formatting- Responsibilities: Path utilities, metadata helpers
Function-Level Breakdown
Workflow Fetching Domain
- Lines 36-47:
FetchWorkflowFromSource()- ~12 lines (orchestration) - Lines 49-66:
fetchLocalWorkflow()- ~18 lines - Lines 68-157:
fetchRemoteWorkflow()- ~90 lines (largest, handles GitHub API resolution)
Include Fetching Domain
- Lines 158-252:
FetchIncludeFromSource()- ~95 lines - Lines 254-300:
fetchAndSaveRemoteFrontmatterImports()- ~47 lines - Lines 302-472:
fetchFrontmatterImportsRecursive()- ~171 lines (recursive, complex) - Lines 474-577:
fetchAndSaveRemoteIncludes()- ~104 lines
Dispatch Workflow Domain
- Lines 628-677:
extractDispatchWorkflowNames()- ~50 lines - Lines 679-850:
fetchAndSaveRemoteDispatchWorkflows()- ~172 lines - Lines 1078-1255:
fetchAndSaveDispatchWorkflowsFromParsedFile()- ~178 lines
Resource Domain
- Lines 852-899:
extractResources()- ~48 lines - Lines 901-1076:
fetchAndSaveRemoteResources()- ~176 lines
Utilities
- Lines 579-616:
getParentDir()+readSourceRepoFromFile()- ~38 lines - Lines 618-627:
sourceRepoLabel()- ~10 lines
Refactoring Strategy
Proposed File Splits
Based on functional domain analysis, split the file into the following modules:
-
fetch.go(Core workflow fetching)- Public:
FetchWorkflowFromSource(),FetchedWorkflowtype - Private:
fetchLocalWorkflow(),fetchRemoteWorkflow() - Estimated LOC: 180-200
- Responsibility: High-level workflow fetching orchestration
- Public:
-
includes.go(Include file and frontmatter import handling)- Public:
FetchIncludeFromSource() - Private:
fetchAndSaveRemoteFrontmatterImports(),fetchFrontmatterImportsRecursive(),fetchAndSaveRemoteIncludes() - Estimated LOC: 350-400
- Responsibility: Recursive include resolution and import management
- Public:
-
dispatch.go(Dispatch workflow extraction and processing)- Public: None (all functions are internal orchestration)
- Private:
extractDispatchWorkflowNames(),fetchAndSaveRemoteDispatchWorkflows(),fetchAndSaveDispatchWorkflowsFromParsedFile() - Estimated LOC: 300-350
- Responsibility: Dispatch workflow discovery, fetching, and conflict detection
-
resources.go(Resource file handling)- Public: None (internal orchestration)
- Private:
extractResources(),fetchAndSaveRemoteResources() - Estimated LOC: 200-250
- Responsibility: Resource extraction and remote file downloading
-
helpers.go(Utility functions shared across modules)- Public: None (all are private helpers)
- Private:
getParentDir(),readSourceRepoFromFile(),sourceRepoLabel() - Estimated LOC: 50-70
- Responsibility: Path utilities, metadata helpers, string formatting
Dependencies Between Modules
┌─────────────┐
│ fetch.go │ (Core API, references helpers)
└──────┬──────┘
│ (imports)
├─────────────────────┬──────────────────┬──────────────┐
│ │ │ │
helpers.go includes.go dispatch.go resources.go
(includes fetch) (calls fetch) (calls fetch)
- fetch.go: Low coupling, references helpers only
- includes.go: References fetch (for orchestration), helpers
- dispatch.go: References fetch (public entry point), helpers
- resources.go: References fetch (public entry point), helpers
- helpers.go: No dependencies on other modules
Interface Abstractions
Consider introducing interfaces to reduce coupling between modules:
// In helpers.go or a new interfaces.go
type RemoteFetcher interface {
Fetch(owner, repo, ref, path string) ([]byte, error)
}
type ResourceHandler interface {
ExtractResources(content string) ([]string, error)
SaveResources(content, targetDir string, spec *WorkflowSpec, verbose bool, force bool) error
}This allows modules to be tested independently with mock implementations.
Test Coverage Plan
New Test Files
Each new file gets a corresponding test file. Maintain existing test coverage and add new tests for module boundaries:
-
fetch_test.go(Tests for fetch.go)- Existing tests: Migrate 20-25 test cases
- New tests: Test module-level boundaries
- Target coverage: >85%
- Key scenarios:
- Local workflow fetching
- Remote workflow resolution (commit SHA tracking)
- Error handling for missing files
- Verbose output logging
-
includes_test.go(Tests for includes.go)- Existing tests: Migrate 15-20 test cases
- New tests: Recursive import resolution edge cases
- Target coverage: >85%
- Key scenarios:
- Frontmatter import parsing
- Recursive import chains
- Circular dependency detection (if applicable)
- File I/O error handling
- Source tracking through imports
-
dispatch_test.go(Tests for dispatch.go)- Existing tests: Migrate 15-20 test cases
- New tests: Workflow name extraction edge cases
- Target coverage: >85%
- Key scenarios:
- YAML dispatch workflow parsing
- GitHub Actions expression handling
- Conflict detection (same source vs. different sources)
- Skip behavior for expressions
- File tracking (created/modified)
-
resources_test.go(Tests for resources.go)- Existing tests: Migrate 10-15 test cases
- New tests: Resource extraction patterns
- Target coverage: >85%
- Key scenarios:
- Resource path extraction
- Expression syntax detection
- Markdown vs. non-Markdown files
- Force overwrite behavior
- Conflict detection
-
helpers_test.go(Tests for helpers.go)- New tests: All utility functions
- Estimated cases: 8-12
- Target coverage: >90%
- Key scenarios:
- Path manipulation edge cases
- Source repo metadata reading
- Label formatting variations
Migration Strategy
- Start with helpers_test.go (independent tests)
- Move to fetch_test.go (core functionality)
- Then includes_test.go, dispatch_test.go, resources_test.go (interdependent)
- Keep remote_workflow_test.go as integration tests for module boundaries
- Run
make test-unitafter each migration step
Test Organization
- Maintain existing test structure and naming conventions
- Keep table-driven tests for complex scenarios
- Add descriptive test names that indicate the module being tested
- Group related tests with
t.Run()for clarity
Implementation Guidelines
- Preserve Public API: Keep
FetchWorkflowFromSource()andFetchedWorkflowin fetch.go for backward compatibility - Update Imports: All modules import pkg/console, pkg/logger, pkg/parser, pkg/workflow
- Shared State: Move
FetchedWorkflowtype definition to common location (consider helpers.go) - Error Handling: Maintain consistent error wrapping with
fmt.Errorf()and context messages - Logging: Each file has its own logger (e.g.,
fetchLog,includesLog,dispatchLog) - Incremental Changes: Split one module at a time, run tests after each split
Acceptance Criteria
- Original
remote_workflow.gois split into 5 focused files (fetch.go, includes.go, dispatch.go, resources.go, helpers.go) - Each new file is under 400 lines
- All tests pass (
make test-unit) - Test coverage is ≥85% for each new file
- No breaking changes to public API (
FetchWorkflowFromSource,FetchdWorkflow) - Code passes linting (
make lint) - Build succeeds (
make build) - Integration tests verify module boundaries (test cross-file interactions)
Additional Context
Repository Guidelines:
- Follow patterns in
pkg/cli/(refer toaudit_command.go,compile_command.gofor command structure) - Use semantic types from
pkg/constantswhere applicable - Maintain logger convention:
logger.New("cli:module_name")
Code Organization:
- Group related functions in files by responsibility
- Prefer many small files over monolithic files
- Add module-level comments explaining responsibilities
Testing:
- Match existing test patterns in
remote_workflow_test.go - Use table-driven tests with
t.Run()and descriptive names - Include error path testing (network failures, missing files, etc.)
Build & Test:
# After making changes
make fmt # Format code
make test-unit # Run unit tests
make lint # Check for issues
make build # Verify build succeedsBefore Committing:
make agent-finish # Complete validationPriority: Medium
Effort: Large (multiple files, significant test migration)
Expected Impact: Improved code organization, easier to understand and maintain, better test isolation
Generated by Daily File Diet · ◷
- expires on Mar 12, 2026, 1:35 PM UTC