[file-diet] File Diet: Refactor pkg/cli/add_workflow_resolution.go (1214 lines)
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 541
- Avg merge
- 5h 48m
- Merged PRs (30d)
- 773
Description
### Overview
The file `pkg/cli/add_workflow_resolution.go` has grown to **1214 lines**, making it the largest non-test Go source file in the repository and difficult to maintain and test. This task involves refactoring it into smaller, focused files with improved test coverage.
### Current State
- **File**: `pkg/cli/add_workflow_resolution.go`
- **Size**: 1214 lines
- **Test Coverage**: No dedicated `add_workflow_resolution_test.go`. Only two narrowly-scoped test files exist (`add_workflow_resolution_manifest_ref_test.go`, 50 lines; `add_workflow_resolution_redirect_test.go`, 143 lines) — a combined ~193 lines of tests, a **~16% test-to-source ratio**.
- **Complexity**: Contains 5 distinct functional domains (spec parsing/orchestration, workflow spec resolution, local repository package resolution, repository package spec appending, and wildcard/content expansion) spanning ~40 functions and several private struct types, all in a single file.
Full File Analysis
#### Detailed Breakdown
Semantic scan of top-level declarations (`func`/`type`/`var`) shows the file organizes into five clusters by line range:
1. **Lines 1–330 — Top-level orchestration & spec parsing**
- `ResolveWorkflows` (L84) — main entry point
- `validateResolveWorkflowsInput`, `parseWorkflowSpecsForResolution`, `parseSingleWorkflowSpecForResolution`
- `resolveLocalPackageWorkflowSpec`, `resolveRepositoryPackageWorkflowSpec`, `resolveRepositoryPackageFallback`, `resolveRepositoryPackageSpecs`
- `validateCurrentRepositorySpecs`, `expandWorkflowSpecsIfNeeded`, `resolveWorkflowSpecs`
- Types: `ResolvedWorkflow`, `ResolvedWorkflows`, `specResolutionResult`
2. **Lines 314–456 — Single workflow spec resolution**
- `resolveSingleWorkflowSpec`, `resolvePackageOrActionWorkflow`, `resolveStandardWorkflow`
- `validateManifestWorkflowPrivateSetting`, `selectBootstrapProfile`
- Type: `resolvedWorkflowResult`
3. **Lines 457–970 — Local repository package resolution (largest cluster, ~510 lines)**
- `resolveLocalRepositoryPackage`, `resolveLocalPackageProjectFileAndValidateAssets`, `resolveLocalRepositoryPackageProjectFile`, `newResolvedLocalRepositoryPackage`
- `resolveLocalRepositoryPackageManifestNodes`, `expandLocalPackageWildcardIncludes`, `localRepositoryPackageManifest`
- `normalizeLocalPackageInstallablePaths`, `localPackageInstallablesFromScannedPaths`, `validateLocalPackageMappingSource`, `packageInstallableWorkflowName`
- `appendLocalRepositoryPackageWorkflowSpecs`
- Skill/agent file discovery: `resolveLocalPackageSkillFiles`, `collectLocalPackageSkillDirFiles`, `resolveLocalPackageAgentFiles`, `scanLocalPackageSkillDirs`
4. **Lines 971–1062 — Repository package spec appending**
- `appendRepositoryPackageWorkflowSpecs`, `appendRepositoryPackageExtensionSpecs`
5. **Lines 1063–1214 — Content fetching & wildcard expansion**
- `resolveAddWorkflowSpecAndContent`, `expandLocalWildcardWorkflows`, `checkWorkflowHasDispatchFromContent`, `expandLocalWildcard`
Cluster 3 (local repository package resolution, including skill/agent file discovery) is the largest coherent sub-domain and the strongest candidate for its own file(s) — it currently accounts for ~42% of the file's lines.
### Refactoring Strategy
#### Proposed File Splits
Based on the cluster analysis above, split the file into the following modules:
1. **`add_workflow_resolution.go`** (retained, core orchestration)
- Functions: `ResolveWorkflows`, `validateResolveWorkflowsInput`, `parseWorkflowSpecsForResolution`, `parseSingleWorkflowSpecForResolution`, `resolveLocalPackageWorkflowSpec`, `resolveRepositoryPackageWorkflowSpec`, `resolveRepositoryPackageFallback`, `resolveRepositoryPackageSpecs`, `validateCurrentRepositorySpecs`, `expandWorkflowSpecsIfNeeded`, `resolveWorkflowSpecs`
- Types: `ResolvedWorkflow`, `ResolvedWorkflows`, `specResolutionResult`
- Responsibility: Top-level entry point and spec-parsing orchestration
- Estimated LOC: ~330
2. **`add_workflow_resolution_spec.go`**
- Functions: `resolveSingleWorkflowSpec`, `resolvePackageOrActionWorkflow`, `resolveStandardWorkflow`, `validateManifestWorkflowPrivateSetting`, `selectBootstrapProfile`
- Type: `resolvedWorkflowResult`
- Responsibility: Resolving a single workflow spec into fetched content and bootstrap profile
- Estimated LOC: ~130
3. **`add_workflow_resolution_local_package.go`**
- Functions: `resolveLocalRepositoryPackage`, `resolveLocalPackageProjectFileAndValidateAssets`, `resolveLocalRepositoryPackageProjectFile`, `newResolvedLocalRepositoryPackage`, `resolveLocalRepositoryPackageManifestNodes`, `expandLocalPackageWildcardIncludes`, `localRepositoryPackageManifest`, `normalizeLocalPackageInstallablePaths`, `localPackageInstallablesFromScannedPaths`, `validateLocalPackageMappingSource`, `packageInstallableWorkflowName`
- Responsibility: Parsing and validating local repository package manifests and assets
- Estimated LOC: ~330
4. **`add_workflow_resolution_local_package_files.go`**
- Functions: `resolveLocalPackageSkillFiles`, `collectLocalPackageSkillDirFiles`, `resolveLocalPackageAgentFiles`, `scanLocalPackageSkillDirs`, `appendLocalRepositoryPackageWorkflowSpecs`
- Responsibility: Skill/agent file discovery for local packages and appending resulting workflow specs
- Estimated LOC: ~180
5. **`add_workflow_resolution_package_specs.go`**
- Functions: `appendRepositoryPackageWorkflowSpecs`, `appendRepositoryPackageExtensionSpecs`
- Responsibility: Appending resolved repository package specs (workflows and extensions) into the spec list
- Estimated LOC: ~95
6. **`add_workflow_resolution_wildcard.go`**
- Functions: `resolveAddWorkflowSpecAndContent`, `expandLocalWildcardWorkflows`, `checkWorkflowHasDispatchFromContent`, `expandLocalWildcard`
- Responsibility: Fetching workflow content and expanding local wildcard workflow specs
- Estimated LOC: ~150
#### Shared Utilities
No new shared-utility file is required — all helpers reference existing shared types (`WorkflowSpec`, `RepoSpec`, `FetchedWorkflow`, `repositoryPackageManifest*`) already defined elsewhere in `pkg/cli`. Keep those imports intact when splitting.
#### Interface Abstractions
- Consider introducing a small `repositoryPackageResolver` interface around `resolveLocalRepositoryPackage`/`resolveRepositoryPackageSpecs` to decouple local vs. remote resolution paths and simplify testing with mocks, but this is optional and can be deferred to a follow-up.
Test Coverage Plan
Add comprehensive tests for each new file (existing `add_workflow_resolution_manifest_ref_test.go` and `add_workflow_resolution_redirect_test.go` can remain, but their target functions should be reviewed and possibly relocated alongside the code they test):
1. **`add_workflow_resolution_test.go`**
- Test cases: `ResolveWorkflows` end-to-end with local/remote specs, invalid input validation, spec parsing error propagation
- Target coverage: >80%
2. **`add_workflow_resolution_spec_test.go`**
- Test cases: package vs. action vs. standard workflow resolution, manifest private-setting validation, bootstrap profile selection with conflicting profiles
- Target coverage: >80%
3. **`add_workflow_resolution_local_package_test.go`**
- Test cases: manifest node resolution, wildcard include expansion, invalid mapping source rejection, project-file asset validation
- Target coverage: >80%
4. **`add_workflow_resolution_local_package_files_test.go`**
- Test cases: skill/agent directory scanning with nested dirs, missing directories, malformed skill files
- Target coverage: >80%
5. **`add_workflow_resolution_package_specs_test.go`**
- Test cases: appending workflow and extension specs from a resolved repository package, empty package edge cases
- Target coverage: >80%
6. **`add_workflow_resolution_wildcard_test.go`**
- Test cases: wildcard expansion with/without dispatch trigger, content fetch failure handling
- Target coverage: >80%
### Implementation Guidelines
1. **Preserve Behavior**: Ensure all existing functionality works identically
2. **Maintain Exports**: Keep public API unchanged (`ResolveWorkflows`, `ResolvedWorkflow`, `ResolvedWorkflows` must stay exported from `pkg/cli`)
3. **Add Tests First**: Write tests for each new file before refactoring
4. **Incremental Changes**: Split one module at a time
5. **Run Tests Frequently**: Verify `make test-unit` passes after each split
6. **Update Imports**: Ensure all import paths are correct
7. **Document Changes**: Add comments explaining module boundaries
### Acceptance Criteria
- [ ] Original file is split into 6 focused files
- [ ] Each new file is under 500 lines (target under 350)
- [ ] 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`
- **Code Organization**: Prefer many small files grouped by functionality
- **Testing**: Match existing test patterns in `pkg/cli/*_test.go`
---
**Priority**: Medium
**Effort**: Medium (6-way split across a well-clustered but interdependent resolution pipeline; no public API changes expected)
**Expected Impact**: Improved maintainability, easier testing, reduced complexity
> Generated by [🧹 Daily File Diet](https://github.com/github/gh-aw/actions/runs/35098069317) · copilot · auto · 116.5 AIC · ⌖ 8.45 AIC · ⊞ 10.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Fdaily-file-diet%22&type=issues)
> - [x] expires on Sep 18, 2026, 5:02 AM UTC-08:00
Contributor guide
Research direction
Start at ResolveWorkflows in pkg/cli/add_workflow_resolution.go and review the existing manifest-ref and redirect tests in pkg/cli. Split the five functional clusters into the six named files, adding the proposed focused tests while preserving the public API. Done means each file is under 500 lines, tests reach the stated coverage, and make test-unit, make lint, and make build pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- cli
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100