Skip to content

Fix detection job squid crash and propagate Features for cli-proxy pre-pull#25902

Merged
pelikhan merged 2 commits intomainfrom
copilot/fix-detection-job-failures
Apr 12, 2026
Merged

Fix detection job squid crash and propagate Features for cli-proxy pre-pull#25902
pelikhan merged 2 commits intomainfrom
copilot/fix-detection-job-failures

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 12, 2026

Since PR #25868 merged firewall audit/log files into the unified agent artifact, the detection job's squid container crashes on startup (exit code 1) because the artifact download pre-populates /tmp/gh-aw/sandbox/firewall/{logs,audit} with stale files. Separately, buildPullAWFContainersStep wasn't propagating Features from the parent WorkflowData, so the cli-proxy image was silently omitted from the detection job's container pre-pull when that feature flag was enabled.

Changes

  • Clean stale firewall dirs — New buildCleanFirewallDirsStep() runs rm -rf on AWFProxyLogsDir and AWFAuditDir immediately after artifact download, before any containers start:

    // Step 0 in buildDetectionJobSteps, before container pull
    steps = append(steps, c.buildCleanFirewallDirsStep()...)
  • Propagate Features in container pre-pull — Added Features: data.Features to the minimal WorkflowData in buildPullAWFContainersStep(), matching the pattern already used in buildDetectionEngineExecutionStep():

    detectionData := &WorkflowData{
        // ...
        ActionCache: data.ActionCache,
        Features:    data.Features,    // was missing
    }
  • TestsTestCleanFirewallDirsStepPresent, TestCleanFirewallDirsStepOrdering, TestBuildPullAWFContainersStepPropagatesFeatures (verifies cli-proxy image inclusion/exclusion based on feature flag)

Fix two issues causing detection job failures:

1. Add buildCleanFirewallDirsStep to remove stale squid files
   (squid.conf, cache.log, access.log) from sandbox/firewall/logs and
   sandbox/firewall/audit directories that are pre-populated when the
   agent artifact is downloaded. These stale files cause the squid
   container to crash (exit code 1) on startup.

2. Propagate Features field in buildPullAWFContainersStep so the
   cli-proxy feature flag is visible to collectDockerImages, ensuring
   the cli-proxy image is included in the detection job's container
   pre-pull step when the feature is enabled.

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d5bb95e3-39ab-4635-8b56-15a616df5c92

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review April 12, 2026 13:00
Copilot AI review requested due to automatic review settings April 12, 2026 13:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes the threat-detection “detection” job failing under AWF by removing stale firewall audit/log files extracted from the unified agent artifact, and ensures AWF image pre-pull correctly honors Features (so cli-proxy is pulled when enabled).

Changes:

  • Add a new “Clean stale firewall files from agent artifact” step ahead of AWF container pre-pull in detection job steps.
  • Propagate WorkflowData.Features into the minimal WorkflowData used by buildPullAWFContainersStep so feature-gated images (e.g., cli-proxy) are included.
  • Add unit tests covering cleanup step presence/order and feature propagation behavior.
Show a summary per file
File Description
pkg/workflow/threat_detection.go Adds firewall dir cleanup step and propagates Features into AWF pre-pull context.
pkg/workflow/threat_detection_test.go Adds tests for cleanup step presence/order and Features propagation to image pull list.
.github/workflows/workflow-skill-extractor.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/workflow-normalizer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/workflow-health-manager.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/workflow-generator.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/weekly-safe-outputs-spec-review.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/weekly-issue-summary.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/weekly-editors-health-check.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/weekly-blog-post-writer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/video-analyzer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/update-astro.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/unbloat-docs.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/ubuntu-image-analyzer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/typist.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/tidy.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/test-quality-sentinel.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/test-project-url-default.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/test-dispatcher.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/test-create-pr-error-handling.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/terminal-stylist.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/technical-doc-writer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/super-linter.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/sub-issue-closer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/step-name-alignment.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/static-analysis-report.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/stale-repo-identifier.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-workflow-call.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-workflow-call-with-inputs.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-update-cross-repo-pr.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-test-tools.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-temporary-id.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-service-ports.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-project.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-multi-pr.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-gemini.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-create-cross-repo-pr.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-copilot.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-copilot-arm.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-codex.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-claude.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-call-workflow.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-agent-scoped-approved.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-agent-public-none.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-agent-public-approved.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-agent-all-none.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/smoke-agent-all-merged.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/slide-deck-maintainer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/sergo.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/semantic-function-refactor.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/security-review.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/security-compliance.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/scout.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/schema-feature-coverage.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/schema-consistency-checker.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/safe-output-health.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/research.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/repository-quality-improver.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/repo-tree-map.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/repo-audit-analyzer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/release.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/refiner.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/refactoring-cadence.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/q.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/python-data-charts.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/prompt-clustering-analysis.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/pr-triage-agent.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/pr-nitpick-reviewer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/portfolio-analyst.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/poem-bot.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/plan.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/pdf-summary.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/org-health-report.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/notion-issue-summary.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/mergefest.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/mcp-inspector.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/lockfile-stats.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/layout-spec-maintainer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/jsweep.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/issue-triage-agent.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/issue-monster.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/issue-arborist.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/instructions-janitor.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/hourly-ci-cleaner.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/grumpy-reviewer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/gpclean.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/go-pattern-detector.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/go-logger.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/go-fan.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/glossary-maintainer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/github-remote-mcp-auth-test.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/github-mcp-tools-report.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/github-mcp-structural-analysis.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/functional-pragmatist.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/firewall-escape.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/example-workflow-analyzer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/duplicate-code-detector.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/draft-pr-cleanup.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/docs-noob-tester.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/discussion-task-miner.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/dictation-prompt.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/developer-docs-consolidator.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/dev.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/dev-hawk.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/design-decision-gate.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/dependabot-go-checker.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/dependabot-burner.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/delight.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/deep-report.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/dead-code-remover.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-workflow-updater.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-testify-uber-super-expert.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-team-status.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-team-evolution-insights.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-syntax-error-quality.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-semgrep-scan.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-security-red-team.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-secrets-analysis.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-safe-outputs-conformance.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-safe-output-optimizer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-safe-output-integrator.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-repo-chronicle.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-rendering-scripts-verifier.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-regulatory.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-performance-summary.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-otel-instrumentation-advisor.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-observability-report.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-news.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-multi-device-docs-tester.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-mcp-concurrency-analysis.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-issues-report.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-integrity-analysis.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-function-namer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-firewall-report.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-file-diet.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-fact.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-doc-updater.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-doc-healer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-compiler-quality.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-community-attribution.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-code-metrics.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-cli-tools-tester.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-cli-performance.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-choice-test.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-assign-issue-to-user.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/daily-architecture-diagram.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/craft.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/copilot-token-optimizer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/copilot-token-audit.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/copilot-session-insights.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/copilot-pr-prompt-analysis.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/copilot-pr-nlp-analysis.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/copilot-pr-merged-report.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/copilot-cli-deep-research.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/copilot-agent-analysis.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/contribution-check.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/constraint-solving-potd.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/commit-changes-analyzer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/code-simplifier.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/code-scanning-fixer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/cloclo.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/cli-version-checker.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/cli-consistency-checker.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/claude-code-user-docs-review.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/ci-doctor.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/ci-coach.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/breaking-change-checker.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/brave.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/blog-auditor.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/auto-triage-issues.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/audit-workflows.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/artifacts-summary.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/architecture-guardian.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/archie.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/approach-validator.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/api-consumption-report.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/agentic-observability-kit.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/agent-persona-explorer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.
.github/workflows/agent-performance-analyzer.lock.yml Regenerates lockfile to include the new firewall cleanup step before container pre-pull.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 179/179 changed files
  • Comments generated: 2

Comment on lines +185 to 192
// Step 0: Clean stale firewall files left by the agent artifact download.
// The agent artifact populates sandbox/firewall/logs and sandbox/firewall/audit
// with files that cause the squid container to crash on start-up.
steps = append(steps, c.buildCleanFirewallDirsStep()...)

// Step 1: Pull AWF container images - the detection engine runs inside AWF (firewall),
// so pre-pulling the containers speeds up execution and avoids on-demand pulls.
steps = append(steps, c.buildPullAWFContainersStep(data)...)
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new firewall cleanup step runs unconditionally for every detection job run, even when threat detection has engine: false (EngineDisabled) and the job exists only to run custom steps. In that case AWF/squid never starts, so this cleanup is unnecessary and it also deletes /tmp/gh-aw/sandbox/firewall/{logs,audit} from the downloaded agent artifact, which could break custom steps that expect to read those files. Consider gating the cleanup (and possibly the AWF image pre-pull) on the engine actually being enabled, or otherwise avoid deleting these directories when the engine won’t run.

Copilot uses AI. Check for mistakes.
Comment on lines +1448 to +1463
stepsString := strings.Join(steps, "")

cleanIdx := strings.Index(stepsString, "Clean stale firewall files from agent artifact")
guardIdx := strings.Index(stepsString, "Check if detection needed")

if cleanIdx < 0 {
t.Fatal("Expected 'Clean stale firewall files from agent artifact' step")
}
if guardIdx < 0 {
t.Fatal("Expected 'Check if detection needed' step")
}

// The cleanup step must come before the detection guard
if cleanIdx > guardIdx {
t.Error("Cleanup firewall dirs step should appear before detection guard step")
}
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestCleanFirewallDirsStepOrdering currently asserts the cleanup step appears before the detection guard, but the PR description and the implementation intent are specifically about cleaning before the AWF container pull/start. As written, this test would still pass if the cleanup step were accidentally moved after the "Download container images" step. Consider asserting cleanup ordering relative to the pull step as well (e.g., before "Download container images") so the test protects the intended regression.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

Excellent test quality

Metric Value
New/modified tests analyzed 3
✅ Design tests (behavioral contracts) 3 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 2 (67%)
Duplicate test clusters 0
Test inflation detected ⚠️ Yes — 104 test lines vs. 32 production lines (3.25:1 ratio, threshold 2:1)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
TestCleanFirewallDirsStepPresent pkg/workflow/threat_detection_test.go:1411 ✅ Design Happy-path only; no nil/missing config scenarios
TestCleanFirewallDirsStepOrdering pkg/workflow/threat_detection_test.go:1445 ✅ Design Defensive t.Fatal guards; happy-path only
TestBuildPullAWFContainersStepPropagatesFeatures pkg/workflow/threat_detection_test.go:1480 ✅ Design Covers both enabled and disabled feature-flag cases — good edge coverage

Flagged Tests — Requires Review

⚠️ Test Inflation — pkg/workflow/threat_detection_test.go

Issue: 104 lines added to the test file vs. 32 lines added to the production file — a 3.25:1 ratio exceeding the 2:1 guideline.
Root cause: TestCleanFirewallDirsStepPresent and TestCleanFirewallDirsStepOrdering share nearly identical setup boilerplate (NewCompiler(), buildDetectionJobSteps(), strings.Join()) and could be merged into a single table-driven test.
Suggested improvement: Consolidate the two firewall-cleanup tests into one TestCleanFirewallDirs with sub-tests (using t.Run) for presence and ordering, and add rows for edge cases such as SafeOutputs: nil and ThreatDetection: nil.

💡 TestCleanFirewallDirsStepPresent — Missing edge/nil cases

Classification: Design test
Observation: Only tests the happy path where ThreatDetection is configured. There is no assertion on what happens when SafeOutputs is nil or when ThreatDetection is nil (i.e., threat detection not configured at all). If the cleanup step should be unconditional, a test with an empty WorkflowData{} would confirm that.
What would break if deleted? A regression that removes the cleanup step from the generated YAML would go undetected.
Suggested improvement: Add at least one case with minimal/nil config to confirm whether the step is always emitted or only when threat detection is enabled.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 3 tests — unit (//go:build !integration)
  • 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). All three new tests verify observable behavioral contracts of the detection job compiler. The test inflation flag is advisory — the tests add genuine value and the ratio is driven by overlapping boilerplate rather than low-signal assertions.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 835.9K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 80/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All three new tests (TestCleanFirewallDirsStepPresent, TestCleanFirewallDirsStepOrdering, TestBuildPullAWFContainersStepPropagatesFeatures) verify observable behavioral contracts. Advisory: minor test inflation (3.25:1) and missing nil/edge cases noted in the review comment.

@github-actions
Copy link
Copy Markdown
Contributor

Commit pushed: c846d08

🏗️ ADR gate enforced by Design Decision Gate 🏗️

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ Design Decision Gate — ADR Required

This PR makes significant changes to core business logic (136 new lines in pkg/workflow/) but does not have a linked Architecture Decision Record (ADR).

AI has analyzed the PR diff and generated a draft ADR to help you get started:

📄 Draft ADR: docs/adr/25902-pre-clean-firewall-dirs-and-propagate-features-in-detection-job.md

The draft covers two design decisions identified in the diff:

  1. Pre-cleaning stale firewall directories — the choice to rm -rf AWFProxyLogsDir/AWFAuditDir as step 0 in the detection job (rather than excluding those paths from the artifact, reconfiguring squid, or restructuring artifact extraction)
  2. Propagating Features in buildPullAWFContainersStep — the choice to align this function with the pattern already used in buildDetectionEngineExecutionStep

What to do next

  1. Review the draft ADR committed to your branch — it was generated from the PR diff
  2. Complete the missing sections — add context the AI couldn't infer (e.g. why alternative approaches were ruled out, any constraints that made this the right call), and adjust the decision rationale if needed
  3. Commit the finalized ADR to docs/adr/ on your branch
  4. Reference the ADR in this PR body by adding a line such as:

    ADR: ADR-25902: Pre-Clean Stale Firewall Directories and Propagate Features in Detection Job

Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision.

Why ADRs Matter

"AI made me procrastinate on key design decisions. Because refactoring was cheap, I could always say 'I'll deal with this later.' Deferring decisions corroded my ability to think clearly."

ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you.


📋 Michael Nygard ADR Format Reference

An ADR must contain these four sections to be considered complete:

  • Context — What is the problem? What forces are at play?
  • Decision — What did you decide? Why?
  • Alternatives Considered — What else could have been done?
  • Consequences — What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number (e.g., 25902-...md for PR #25902).

🔒 This PR cannot merge until an ADR is linked in the PR body.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · ● 189.7K ·

@pelikhan pelikhan merged commit 25c3669 into main Apr 12, 2026
@pelikhan pelikhan deleted the copilot/fix-detection-job-failures branch April 12, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants