-
Notifications
You must be signed in to change notification settings - Fork 295
Description
Analysis of all 567 non-test Go files across pkg/ identified a cluster of semantically misplaced functions in two files: pkg/workflow/agentic_engine.go and pkg/workflow/js.go. These outliers reduce navigability and create unnecessary coupling between unrelated concerns.
Analysis Metadata
- Total Go files analyzed: 567 (273 in
pkg/workflow/, 204 inpkg/cli/, 40 inpkg/parser/, 50 across small utility packages) - Packages inspected:
workflow,cli,parser,stringutil,console,gitutil,fileutil,sliceutil,mathutil - Total function clusters identified: 8 primary naming groups (
generate*,extract*,parse*,validate*,compute*,build*,render*,format*) - Outliers found: 4 concrete functions in wrong files + 1 redundant wrapper
- Duplicates detected: 1 near-duplicate pattern (
findGitRootacrosspkg/cliandpkg/workflow) - Detection method: Naming-pattern analysis + cross-file reference tracing via Serena LSP + build-constraint-aware deduplication
- Workflow run: §23062067331
Identified Issues
1. YAML Utility Functions Stranded in agentic_engine.go
File: pkg/workflow/agentic_engine.go (642 lines)
Functions: ConvertStepToYAML (line 562), unquoteUsesWithComments (line ~590)
Issue: These are YAML marshalling helpers that have no logical relationship to engine base classes or the EngineRegistry. Their presence in agentic_engine.go forced the creation of a one-line wrapper method in compiler_yaml_helpers.go:
// compiler_yaml_helpers.go:60
// This is a method wrapper around the package-level ConvertStepToYAML function.
func (c *Compiler) convertStepToYAML(stepMap map[string]any) (string, error) {
return ConvertStepToYAML(stepMap)
}
```
The existence of this wrapper is a code smell indicating the original function is in the wrong file.
**Callers**:
- `pkg/workflow/compiler_yaml_main_job.go:306` — calls `ConvertStepToYAML` directly
- `pkg/workflow/compiler_yaml_helpers.go:61` — single-line wrapper around the same function
**Recommendation**: Move `ConvertStepToYAML` and `unquoteUsesWithComments` into `pkg/workflow/compiler_yaml_helpers.go`, promoting the wrapper to the canonical implementation and eliminating the indirection.
---
#### 2. Playwright Tool List Stranded in `agentic_engine.go`
**File**: `pkg/workflow/agentic_engine.go` (line 527)
**Function**: `GetPlaywrightTools() []any`
**Issue**: Returns a hardcoded list of Playwright browser tool names (`browser_click`, `browser_navigate`, etc.). This has no relationship to the engine base type, registry, or lifecycle — it is a static configuration constant masquerading as a function.
**Callers**:
- `pkg/workflow/claude_tools.go:105` — uses it to populate Claude's allowed tool list
- `pkg/workflow/codex_engine.go:440` — uses it to populate Codex's allowed tool list
Neither caller is the engine base or registry. The function sits between engine infrastructure and Playwright tool constants with no clear ownership.
**Recommendation**: Move `GetPlaywrightTools()` to `pkg/workflow/playwright.go` (or create `pkg/workflow/playwright_tools.go`). Alternatively, promote the list to a `var` constant adjacent to Playwright tool configuration.
---
#### 3. Cross-Engine Step Generator Stranded in `agentic_engine.go`
**File**: `pkg/workflow/agentic_engine.go` (line 484)
**Function**: `GenerateMultiSecretValidationStep(secretNames []string, engineName, docsURL string, envOverrides map[string]string) GitHubActionStep`
**Issue**: This function generates a GitHub Actions step that validates secrets are available. It is conceptually identical in purpose to functions in `pkg/workflow/runtime_step_generator.go`:
```
runtime_step_generator.go:
GenerateRuntimeSetupSteps(requirements []RuntimeRequirement) []GitHubActionStep
generateEnvCaptureStep(envVar string, captureCmd string) GitHubActionStep
GenerateSerenaLanguageServiceSteps(tools *ToolsConfig) []GitHubActionStep
generateSetupStep(req *RuntimeRequirement) GitHubActionStepCallers (all separate engine files — not the engine base itself):
pkg/workflow/claude_engine.go:76pkg/workflow/gemini_engine.go:89pkg/workflow/codex_engine.go:84pkg/workflow/copilot_engine_installation.go:37
Recommendation: Move GenerateMultiSecretValidationStep to pkg/workflow/runtime_step_generator.go, co-locating it with the other step-generator functions it semantically matches.
4. Safe Output Tool Configuration Embedded in js.go
File: pkg/workflow/js.go
Functions: GetSafeOutputsToolsJSON() string (line 91), GetSafeOutputToolOptions() []SafeOutputToolOption (line 33), SafeOutputToolOption struct (line 13)
Issue: js.go is a JavaScript utilities file whose primary concerns are JavaScript source embedding, comment stripping, and YAML formatting. However, it also:
- Embeds
js/safe_outputs_tools.jsonvia//go:embed - Defines the
SafeOutputToolOptionstruct - Exposes
GetSafeOutputsToolsJSON()(consumed bysafe_outputs_tools_filtering.go:32) - Exposes
GetSafeOutputToolOptions()(consumed bypkg/cli/interactive.go:543)
The safe outputs JSON embed ended up here, pulling these functions along. The result is that safe-output-related symbols are scattered between js.go, safe_outputs_config.go, safe_outputs_tools_filtering.go, and safe_outputs_config_helpers.go.
Recommendation: Create pkg/workflow/safe_outputs_tools.go and move:
- The
//go:embed js/safe_outputs_tools.jsondirective SafeOutputToolOptionstructGetSafeOutputsToolsJSON()GetSafeOutputToolOptions()
This isolates the safe-outputs JSON concern from JavaScript concerns.
5. Near-Duplicate findGitRoot Across Packages
Occurrences:
pkg/cli/git.go:25: func findGitRoot() (string, error)— returns errorpkg/workflow/git_helpers.go:54: func findGitRoot() string— swallows errors, returns empty string
Both functions walk up the directory tree to find the .git root directory. They are in separate packages so they're not technically duplicates, but the logic is similar.
Recommendation: Consider moving the canonical implementation into pkg/gitutil/ (the package already exists) and having both consumers call the shared implementation. This is lower priority than issues 1–4 since the functions are unexported and well-contained.
Refactoring Recommendations
Priority 1: High Impact / Low Effort
| # | Action | Source | Destination | Impact |
|---|---|---|---|---|
| 1 | Move ConvertStepToYAML + unquoteUsesWithComments |
agentic_engine.go |
compiler_yaml_helpers.go |
Eliminates one-line wrapper, co-locates YAML utilities |
| 2 | Move GetPlaywrightTools |
agentic_engine.go |
playwright.go or new playwright_tools.go |
Removes unrelated browser-tool logic from engine base |
| 3 | Move GenerateMultiSecretValidationStep |
agentic_engine.go |
runtime_step_generator.go |
Consolidates all step-generator functions in one file |
Priority 2: Medium Effort
| # | Action | Source | Destination | Impact |
|---|---|---|---|---|
| 4 | Migrate safe-output JSON embed + functions | js.go |
new safe_outputs_tools.go |
Untangles JS utilities from safe-outputs config |
Priority 3: Long-term
| # | Action | Details |
|---|---|---|
| 5 | Consolidate findGitRoot |
Move shared implementation to pkg/gitutil, update callers in cli and workflow |
Implementation Checklist
- Move
ConvertStepToYAMLandunquoteUsesWithCommentsfromagentic_engine.gotocompiler_yaml_helpers.go; delete the wrapper method - Move
GetPlaywrightToolsfromagentic_engine.gotoplaywright.go(or newplaywright_tools.go) - Move
GenerateMultiSecretValidationStepfromagentic_engine.gotoruntime_step_generator.go - Create
safe_outputs_tools.go; move JSON embed directive,SafeOutputToolOption,GetSafeOutputsToolsJSON, andGetSafeOutputToolOptionsfromjs.go - Verify
make test-unitpasses after each change - Verify
make buildandmake lintpass
References:
Generated by Semantic Function Refactoring · ◷
- expires on Mar 15, 2026, 5:18 PM UTC