[refactor] Consolidate duplicated agent-output resolution + safe-output item parsing in pkg/cli

[github-actions]

🔧 Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw — pkg/ source tree (excluding *_test.go and testdata/)

Overview

Re-scanned the pkg/ tree (917 non-test .go files, ~5.2k top-level functions/methods), clustering functions by name and purpose and verifying candidates by reading both bodies. Consistent with the prior pass, code organization is strong: the one-feature-per-file convention holds (codemod_*.go, create_*, extract_* each in their own file), shared string utilities live in pkg/stringutil, and most same-name collisions are intentional (*_wasm.go build-tag twins, thin re-export wrappers). Those are not flagged here.

This pass surfaces one genuine, high-value duplication cluster that the previous issue (#39167, now closed) did not cover.

Key Findings

No.	Finding	Severity	Scope
1	Agent-output resolution + safe-output items parsing copy-pasted across 4 functions	Medium-High	pkg/cli — 2 files
2	The three extract*FromRun functions differ only by item struct + type filter	Medium	pkg/cli/logs_metrics.go

(Findings 1 and 2 are two views of the same cluster — one fix addresses both.)

Finding 1 — Duplicated agent-output resolution & item parsing (Priority 1)

Four functions independently re-implement the same three-step pattern: (a) resolve agent_output.json across the new flattened form and the old nested-directory form (with recursive fallback), (b) read + json.Unmarshal into { Items []json.RawMessage; Errors []string }, (c) loop the items, unmarshal each into a per-type struct, and filter on item.Type.

Function	File:line	Terminal action
extractMissingToolsFromRun	pkg/cli/logs_metrics.go:260	collect missing_tool items
extractNoopsFromRun	pkg/cli/logs_metrics.go:386	collect noop items
extractMissingDataFromRun	pkg/cli/logs_metrics.go:508	collect missing_data items
runContainsSafeOutputType	pkg/cli/logs_run_processor.go:451	return bool on first type match

The resolution block at pkg/cli/logs_metrics.go:267-308 is byte-identical across the three logs_metrics.go functions (repeated at lines 393 and 515); runContainsSafeOutputType carries a simplified variant of the same logic. findAgentOutputFile (pkg/cli/logs_utils.go:94) already factors out the recursive search, but the surrounding new-vs-old-form resolution and the read/parse/loop scaffolding are inlined four times. Only the per-item struct fields and the item.Type filter vary between functions.

Shared resolution + parse scaffolding (logs_metrics.go:267-334, repeated at :393 and :515)

agentOutputJSONPath := filepath.Join(runDir, constants.AgentOutputFilename)

var resolvedAgentOutputFile string
if stat, err := os.Stat(agentOutputJSONPath); err == nil && !stat.IsDir() {
    resolvedAgentOutputFile = agentOutputJSONPath
} else {
    agentOutputPath := filepath.Join(runDir, constants.AgentOutputArtifactName)
    if stat, err := os.Stat(agentOutputPath); err == nil {
        if stat.IsDir() {
            nested := filepath.Join(agentOutputPath, constants.AgentOutputArtifactName)
            if _, nestedErr := os.Stat(nested); nestedErr == nil {
                resolvedAgentOutputFile = nested
            }
        } else {
            resolvedAgentOutputFile = agentOutputPath
        }
    } else if found, ok := findAgentOutputFile(runDir); ok {
        resolvedAgentOutputFile = found
    }
}

if resolvedAgentOutputFile != "" {
    cleanPath := filepath.Clean(resolvedAgentOutputFile)
    content, readErr := os.ReadFile(cleanPath)
    if readErr != nil { /* verbose + return */ }

    var safeOutput struct {
        Items  []json.RawMessage `json:"items"`
        Errors []string          `json:"errors,omitempty"`
    }
    if err := json.Unmarshal(content, &safeOutput); err != nil { /* verbose + return */ }

    for _, itemRaw := range safeOutput.Items {
        var item struct { /* the ONLY part that varies between functions */ }
        if err := json.Unmarshal(itemRaw, &item); err != nil { continue }
        // type-specific filter + append (the other part that varies)
    }
}

Note: extractMCPFailuresFromRun (logs_metrics.go:636) is not part of this cluster — it walks raw .log files and is correctly kept separate.

Recommendation

1. Extract the new-vs-old-form resolution into a helper alongside findAgentOutputFile, e.g. resolveAgentOutputFile(runDir string, verbose bool) (string, bool).
2. Extract the read + unmarshal-items step into a generic iterator, e.g. func forEachSafeOutputItem[T any](runDir string, verbose bool, fn func(T)) error, so each extract*FromRun reduces to ~10 lines (resolve → iterate → map the matching type into its report). runContainsSafeOutputType becomes a short-circuiting caller of the same iterator.

Estimated impact: removes ~200 lines of copy-pasted resolution/parse boilerplate and gives the agent-output format a single source of truth — the three backward-compat branches currently must be edited in four places, so they risk drifting apart.

Acceptable patterns confirmed (NOT findings)

• *_wasm.go build-tag twins — parallel implementations behind build constraints. Correct.
• codemod_*.go one-per-file — intentional; each codemod is its own unit. Not scatter.
• Thin re-export wrappers (e.g. cli / parser forwarders to stringutil) — correct re-exports.
• extractMCPFailuresFromRun — distinct log-walking logic, correctly separate from the cluster above.

Next Actions

• Add resolveAgentOutputFile(runDir, verbose) (string, bool) next to findAgentOutputFile in pkg/cli/logs_utils.go.
• Add a generic forEachSafeOutputItem[T] (or parseSafeOutputItems) helper and route the three logs_metrics.go extract*FromRun functions and runContainsSafeOutputType through it.
• Verify behavior parity with existing pkg/cli tests (logs_metrics, logs_run_processor).

Analysis Metadata

• Non-test Go files scanned: 917 (pkg/, excluding *_test.go and testdata/)
• Primary packages: pkg/workflow (400), pkg/cli (319), pkg/parser (43)
• Genuine duplication clusters found: 1 (4 functions, 2 files)
• Detection method: name/purpose clustering + manual body-by-body verification
• Prior issue [refactor] Semantic clustering: extract duplicated linter AST helpers + copilot-perms predicate #39167 (linter AST helpers + copilot-perms predicate) closed and superseded by this pass.

Generated by 🔧 Semantic Function Refactoring · 359.4 AIC · ⌖ 14.3 AIC · ⊞ 8.6K · ◷

• expires on Jun 16, 2026, 4:17 PM UTC-08:00

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · 39.4 AIC · ⌖ 4.39 AIC · ⊞ 22K · ◷

[copilot-swe-agent]

The agent encountered an error and was unable to start working on this issue: Please try again later, or contact support if the problem persists. (Request id: 1600:2C9333:10B309C:3D7DF71:6A2F63EA)

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · 33.4 AIC · ⌖ 4.34 AIC · ⊞ 22.7K · ◷

[github-actions]

Closing this issue as a new semantic function clustering and refactoring analysis is being performed. Findings will be consolidated into a fresh report.