Skip to content

[file-diet] Refactor pkg/workflow/threat_detection.go (1542 lines) into focused modules #41224

Description

@github-actions

Overview

The file pkg/workflow/threat_detection.go has grown to 1542 lines, making it difficult to navigate and maintain. This task involves splitting it into focused, cohesive modules while preserving all behavior and public APIs.

Note: pkg/parser/remote_fetch.go is also 1542 lines (tied for largest). A follow-up issue should address that file separately.

Current State

  • File: pkg/workflow/threat_detection.go
  • Size: 1542 lines
  • Test coverage: Excellent — 3,650 total test lines across 5 test files (ratio ≈ 2.37×)
    • threat_detection_test.go (2235 lines)
    • threat_detection_job_combinations_integration_test.go (684 lines)
    • threat_detection_isolation_test.go (395 lines)
    • threat_detection_file_access_test.go (229 lines)
    • threat_detection_conclude_script_test.go (107 lines)
Full File Analysis

Symbol Distribution (from semantic analysis)

Category Count Examples
*Compiler methods 27 buildDetectionJob, buildDetectionEngineExecutionStep
Standalone functions 11 IsDetectionJobEnabled, engineCoreSecretVarNames
*ThreatDetectionConfig methods 3 IsContinueOnError, HasRunnableDetection, IsConditional
Structs 1 ThreatDetectionConfig (11 fields)
Constants 2 detectionStepCondition, stepEnvIndent

Complexity Hotspots (by function length)

Function Lines Domain
buildDetectionEngineExecutionStep ~167 Engine config + script generation
buildDetectionJob ~140 Job orchestration (top-level)
buildExternalDetectorExecutionStep ~121 External runner env/args
buildDetectionJobSteps ~101 Step orchestration
parseThreatDetectionObjectConfig ~74 Config parsing
buildInstallDetectionEngineForExternalDetectorStep ~60 External installer

Logical Structure (by line range)

  • Lines 1–260: Config struct, parsing helpers, simple predicates
  • Lines 260–555: Step orchestration + core guard/prepare/cleanup steps
  • Lines 556–865: Analysis, conclusion, token-summary, engine execution steps
  • Lines 866–1402: External detector: engine ID, install, run, conclude, workspace checkout
  • Lines 1403–1542: Job builder (buildDetectionJob)

Refactoring Strategy

Proposed File Splits

  1. threat_detection_config.go (~260 lines)

    • Struct: ThreatDetectionConfig (all fields)
    • Methods: IsContinueOnError, HasRunnableDetection, IsConditional
    • Methods: parseThreatDetectionConfig, parseThreatDetectionObjectConfig
    • Function: extractRawExpression
    • Responsibility: Data model + config parsing only
  2. threat_detection_helpers.go (~140 lines)

    • Functions: IsDetectionJobEnabled, IsConditionalDetection, isThreatDetectionExplicitlyDisabledInConfigs
    • Functions: getThreatDetectionAdditionalAllowedDomains, canReuseThreatDetectionEngineConfigForExternalDetector, engineCoreSecretVarNames
    • Variable: threatLog; Constants: detectionStepCondition, stepEnvIndent
    • Responsibility: Stateless predicates and utility helpers
  3. threat_detection_steps.go (~460 lines)

    • Methods: buildDetectionJobSteps, buildDetectionGuardStep, buildClearMCPConfigStep, buildCleanFirewallDirsStep, buildPrepareDetectionFilesStep
    • Methods: buildDetectionConclusionStep, buildDetectionTokenUsageSummaryStep, buildThreatDetectionAnalysisStep
    • Methods: buildSetupScriptRequire, buildResultsParsingScriptRequire, buildWorkflowContextEnvVars
    • Methods: buildUploadDetectionLogStep, buildCustomThreatDetectionSteps, buildInstallThreatDetectStep
    • Responsibility: Step builders for the standard (non-external) detection flow
  4. threat_detection_external.go (~470 lines)

    • Methods: buildPrepareDetectionEngineConfigForExternalDetectorStep, buildPullAWFContainersStep
    • Methods: getExternalThreatDetectionEngineID, getThreatDetectionEngineID
    • Functions: buildExternalDetectorCodexConfig, codexProxyWebsocketBaseURL
    • Methods: buildInstallAWFForExternalDetectorStep, buildInstallDetectionEngineForExternalDetectorStep
    • Function: isAWFBinaryInstallStep
    • Methods: buildExternalDetectorExecutionStep, buildUploadDetectionArtifactStep, buildExternalDetectorConcludeStep, buildWorkspaceCheckoutForDetectionStep
    • Function: extractStepEnvLines
    • Responsibility: All external-detector-specific install, run, and conclude logic
  5. threat_detection_job.go (~210 lines)

    • Method: buildDetectionJob (top-level job orchestrator)
    • Responsibility: Assembles the complete detection job from the above modules

Interface Abstractions

Consider extracting an interface to decouple engine-specific config generation:

type ThreatDetectionEngine interface {
    EngineID() string
    InstallSteps(data *WorkflowData) []string
    ExecutionSteps(data *WorkflowData) []string
}

This would make it easier to add new engine types without modifying threat_detection_external.go.

Test Coverage Plan

Existing test files already cover the file comprehensively. After the split, verify each test still compiles and passes without modification (no test changes should be required). However, consider adding focused unit tests for any currently untested internal helpers:

  1. threat_detection_config_test.go (if needed)

    • Test cases: config parsing edge cases, extractRawExpression with various expression forms
    • Target: cover parseThreatDetectionObjectConfig branches
  2. threat_detection_helpers_test.go (if needed)

    • Test cases: isThreatDetectionExplicitlyDisabledInConfigs with empty/nil/mixed inputs
    • Test cases: getThreatDetectionAdditionalAllowedDomains domain list correctness
  3. threat_detection_external_test.go (if needed)

    • Test cases: buildExternalDetectorCodexConfig output format
    • Test cases: codexProxyWebsocketBaseURL URL transformation

Implementation Guidelines

  1. Preserve Behavior: Ensure all existing functionality works identically after each split
  2. Maintain Exports: Keep all exported functions/types in the same package (package workflow)
  3. Incremental Changes: Split one module at a time; all files stay in pkg/workflow/
  4. Run Tests Frequently: Verify make test-unit passes after each split
  5. No Import Path Changes: Since all files remain in pkg/workflow, no caller updates needed
  6. Document Module Boundaries: Add a one-line package-level comment to each new file

Acceptance Criteria

  • pkg/workflow/threat_detection.go is split into 5 focused files (see above)
  • Each new file is under 500 lines
  • All tests pass (make test-unit)
  • Build succeeds (make build)
  • Linting passes (make lint)
  • No breaking changes to the public API (IsDetectionJobEnabled, IsConditionalDetection, ThreatDetectionConfig)
Additional Context
  • pkg/parser/remote_fetch.go is also 1542 lines and is a candidate for a follow-up refactoring issue
  • All proposed new files remain in pkg/workflow/ — no import path changes required
  • The existing test suite is excellent (2.37× ratio) and should continue passing without modification
  • Follow patterns in pkg/workflow/ — many files follow the create_*.go and domain-specific naming conventions

Priority: Medium
Effort: Medium (file split only — no logic changes required)
Expected Impact: Improved navigability, faster PR reviews, cleaner separation of concerns

Generated by 🧹 Daily File Diet · 76.2 AIC · ⌖ 10.9 AIC · ⊞ 6.9K ·

  • expires on Jun 26, 2026, 5:31 AM UTC-08:00

Metadata

Metadata

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions