ensure firewall access.log artifact uploads even when agent fails early#40360
ensure firewall access.log artifact uploads even when agent fails early#40360Copilot wants to merge 3 commits into
Conversation
- In generateFirewallLogParsingStep: add sudo mkdir -p for the logs and
audit directories so they always exist even when awf exits before
writing any proxy logs (e.g. ERR_CONFIG startup failure)
- In generateSummarySteps: emit a dedicated 'Upload Firewall Logs'
artifact step (if: always(), name: firewall-logs-{workflow}) for every
firewall-enabled workflow, separate from the unified agent artifact
- Add/update tests: TestGenerateFirewallLogParsingStepFixesFirewallPermissions
now checks for the mkdir -p command; new
TestGenerateSummaryStepsIncludesFirewallLogsUpload verifies the
dedicated upload step is present when firewall is enabled
- Update wasm golden fixtures to reflect new step output
- Recompile all 250 workflows
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
There was a problem hiding this comment.
Pull request overview
This PR improves the reliability and discoverability of firewall (Squid) proxy logs in firewall-enabled gh-aw runs, especially for early failures where egress diagnostics are most valuable. It ensures the firewall log/audit directories exist before permission fixes, and emits a dedicated, always-run firewall logs artifact upload step so proxy logs are easier to find even when the main agent flow fails early.
Changes:
- Ensure
/tmp/gh-aw/sandbox/firewall/logsand/tmp/gh-aw/sandbox/firewall/auditare created before firewall permission fixes, so artifact uploads can capture them even when AWF exits before writing logs. - Emit a dedicated
Upload Firewall Logsstep (runs withif: always()) for all firewall-enabled workflows, producing a separatefirewall-logs-*artifact. - Regenerate golden/lock fixtures to reflect the new steps across workflows.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/engine_firewall_support.go | Ensures firewall log/audit directories exist before chmod; defines the firewall logs upload step. |
| pkg/workflow/compiler_yaml_main_job.go | Adds a dedicated always-run firewall logs artifact upload step for firewall-enabled workflows. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/pi.golden | Golden output updated to include mkdir and dedicated firewall logs upload step. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/gemini.golden | Golden output updated to include mkdir and dedicated firewall logs upload step. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden | Golden output updated to include mkdir and dedicated firewall logs upload step. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/codex.golden | Golden output updated to include mkdir and dedicated firewall logs upload step. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/claude.golden | Golden output updated to include mkdir and dedicated firewall logs upload step. |
| .github/workflows/workflow-normalizer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/workflow-health-manager.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/workflow-generator.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/weekly-safe-outputs-spec-review.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/weekly-issue-summary.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/video-analyzer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/update-astro.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/unbloat-docs.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/uk-ai-operational-resilience.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/ubuntu-image-analyzer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/typist.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/tidy.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/test-workflow.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/test-quality-sentinel.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/test-project-url-default.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/test-dispatcher.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/terminal-stylist.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/technical-doc-writer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/super-linter.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/sub-issue-closer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/step-name-alignment.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/static-analysis-report.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/stale-pr-cleanup.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/spec-librarian.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/spec-extractor.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/spec-enforcer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-workflow-call.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-test-tools.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-temporary-id.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-service-ports.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-project.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-pi.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-opencode.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-multi-pr.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-gemini.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-crush.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-copilot.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-codex.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-claude.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-ci.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-call-workflow.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-antigravity.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-agent-public-none.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-agent-public-approved.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/smoke-agent-all-none.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/sergo.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/semantic-function-refactor.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/security-review.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/security-compliance.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/scout.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/schema-feature-coverage.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/schema-consistency-checker.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/safe-output-health.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/ruflo-backed-task.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/research.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/repository-quality-improver.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/repo-tree-map.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/repo-audit-analyzer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/release.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/refiner.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/refactoring-cadence.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/q.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/python-data-charts.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/prompt-clustering-analysis.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/pr-triage-agent.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/pr-sous-chef.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/pr-description-caveman.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/pr-code-quality-reviewer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/portfolio-analyst.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/plan.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/pdf-summary.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/outcome-collector.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/org-health-report.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/objective-impact-report.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/notion-issue-summary.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/necromancer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/metrics-collector.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/mergefest.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/mcp-inspector.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/lockfile-stats.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/linter-miner.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/lint-monster.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/layout-spec-maintainer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/jsweep.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/issue-triage-agent.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/issue-monster.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/issue-arborist.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/instructions-janitor.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/hourly-ci-cleaner.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/hippo-embed.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/grumpy-reviewer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/gpclean.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/go-pattern-detector.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/go-logger.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/go-fan.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/glossary-maintainer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/github-remote-mcp-auth-test.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/functional-pragmatist.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/firewall.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/firewall-escape.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/example-workflow-analyzer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/example-permissions-warning.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/example-failure-category-filter.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/duplicate-code-detector.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/draft-pr-cleanup.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/docs-noob-tester.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/dictation-prompt.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/dev.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/dev-hawk.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/designer-drift-audit.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/design-decision-gate.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/deployment-incident-monitor.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/dependabot-worker.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/dependabot-repair.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/dependabot-campaign.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/dependabot-burner.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/delight.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/deep-report.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/dead-code-remover.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-workflow-updater.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-windows-terminal-integration-builder.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-team-status.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-syntax-error-quality.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-skill-optimizer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-sentrux-report.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-security-red-team.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-secrets-analysis.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-regulatory.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-performance-summary.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-news.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-model-inventory.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-max-ai-credits-test.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-malicious-code-scan.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-issues-report.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-geo-optimizer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-function-namer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-file-diet.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-fact.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-experiment-report.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-doc-updater.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-doc-healer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-credit-limit-test.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-compiler-quality.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-community-attribution.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-cli-performance.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-choice-test.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-byok-ollama-test.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/daily-assign-issue-to-user.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/craft.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/copilot-session-insights.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/copilot-pr-merged-report.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/copilot-opt.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/copilot-cli-deep-research.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/copilot-centralization-optimizer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/copilot-centralization-drilldown.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/copilot-agent-analysis.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/contribution-check.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/constraint-solving-potd.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/codex-github-remote-mcp-test.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/code-simplifier.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/code-scanning-fixer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/cloclo.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/cli-version-checker.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/cli-consistency-checker.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/ci-doctor.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/ci-coach.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/chaos-pr-bundle-fuzzer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/changeset.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/breaking-change-checker.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/brave.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/bot-detection.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/blog-auditor.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/avenger.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/audit-workflows.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/artifacts-summary.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/architecture-guardian.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/archie.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/approach-validator.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/api-consumption-report.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/ai-moderator.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/agentic-token-optimizer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/agent-performance-analyzer.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/ace-editor.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
| .github/workflows/ab-testing-advisor.lock.yml | Recompiled lock workflow includes mkdir and dedicated firewall logs upload step. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 262/262 changed files
- Comments generated: 0
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (>100 new lines) but does not have a linked Architecture Decision Record (ADR). 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
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 ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
🧪 Test Quality Sentinel Report✅ Test Quality Score: 90/100 — Excellent
📊 Metrics & Test Classification (2 tests analyzed)
Go: 1 ( Scoring breakdown:
Verdict
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnose and /tdd — no blocking issues; leaving as COMMENT with improvement suggestions.
📋 Key Themes & Highlights
Key Themes
- Test assertion specificity (2 comments): The new
TestGenerateSummaryStepsIncludesFirewallLogsUploadchecksif: always()andif-no-files-found: ignoreagainst the entire output string rather than the specific upload step block, reducing its regression value. A negative case (firewall disabled) is also missing. - Orphaned engine methods (1 comment):
GetSquidLogsStepson Claude/Copilot/Codex engines anddefaultGetSquidLogsStepsare not called from the compiler. The PR correctly adds the upload step at the compiler level, but the engine-level dead code should be cleaned up to avoid confusion about there being two code paths. - Dedicated artifact scope (1 comment): The new
firewall-logs-*artifact captures onlyAWFProxyLogsDir;AWFAuditDir(policy manifests, squid.conf) is available in the unified agent artifact but not the dedicated one — worth considering for ERR_CONFIG triage.
Positive Highlights
- ✅ Root cause correctly identified and fixed:
mkdir -pbeforechmodensures the upload target always exists, even when AWF exits before writing any proxy traffic. - ✅ The previously dead
generateSquidLogsUploadStepfunction is now correctly wired into the compiler viagenerateSummaryStepsrather than left as dead code. - ✅ Resilient step design: Both the new
mkdir -p(with|| true) and the upload step (if: always()+continue-on-error: true+if-no-files-found: ignore) follow the existing patterns for failure-safe post-agent steps. - ✅ Step order tracking is wired correctly:
RecordArtifactUploadis called after secret redaction is already recorded in the real compilation path. - ✅ 250 workflows recompiled and golden fixtures updated — good mechanical completeness.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 144.5 AIC · ⌖ 9.72 AIC · ⊞ 6.9K
Comments that could not be inline-anchored
pkg/workflow/engine_firewall_support_test.go:318
[/tdd] This if: always() assertion scans the entire output string, not just the "Upload Firewall Logs" block. Since generateFirewallLogParsingStep (emitted earlier in the same output) also contains if: always(), this check would pass even if the upload step was missing that attribute.
<details>
<summary>💡 Suggested fix</summary>
Extract the upload step block from the output before asserting:
uploadIdx := strings.Index(output, "- name: Upload Firewall Logs")
if uploadIdx == -…
</details>
<details><summary>pkg/workflow/engine_firewall_support_test.go:327</summary>
**[/tdd]** The test only covers the positive case (firewall enabled). A negative test — verifying the upload step is NOT emitted when `Firewall.Enabled = false` or `NetworkPermissions` is nil — would lock in the conditional and prevent future regressions where the upload step leaks into non-firewall workflows.
<details>
<summary>💡 Suggested addition</summary>
```go
func TestGenerateSummaryStepsExcludesFirewallLogsUploadWhenDisabled(t *testing.T) {
compiler := NewCompiler()
compiler.s…
</details>
<details><summary>pkg/workflow/compiler_yaml_main_job.go:698</summary>
**[/diagnose]** `GetSquidLogsSteps` is also defined on `ClaudeEngine`, `CopilotEngine`, and `CodexEngine` (all backed by `defaultGetSquidLogsSteps`), but these methods are not part of the `CodingAgentEngine` interface and appear to be unreachable from the compiler. This PR fixes the bug by correctly wiring `generateSquidLogsUploadStep` here, but the engine-level methods are now orphaned dead code that could mislead future maintainers into thinking there are two upload code paths.
<details>
<su…
</details>
<details><summary>pkg/workflow/engine_firewall_support.go:104</summary>
**[/diagnose]** The dedicated "Upload Firewall Logs" artifact only uploads `AWFProxyLogsDir` (squid access logs), not `AWFAuditDir` (policy-manifest.json, squid.conf, docker-compose.redacted.yml). Both directories are created by `mkdir -p` and made readable by `chmod -R`, and the audit dir IS included in the unified agent artifact via `collectArtifactPaths`. However, for ERR_CONFIG early-exit failures — the exact scenario this PR targets — the audit directory content is arguably the most useful…
</details>There was a problem hiding this comment.
The fix is mechanically correct: mkdir -p ensures the log directories exist before the upload step runs, and wiring generateSquidLogsUploadStep into generateSummarySteps gives every firewall-enabled workflow a dedicated firewall-logs-{workflow} artifact with if: always(). The step-order validation (secret redaction before upload) is satisfied by the existing call sequence. Golden fixtures and tests are updated consistently.
Non-blocking concerns (see inline comments)
Orphaned engine methods — latent duplicate-step risk
defaultGetSquidLogsSteps and the three engine GetSquidLogsSteps wrappers (Claude/Copilot/Codex) have no callers anywhere in the codebase. They call both generateSquidLogsUploadStep AND generateFirewallLogParsingStep, which is exactly what generateSummarySteps now does. If those methods are ever wired back in, affected engines will emit duplicate steps silently. The functions should be removed or clearly tombstoned.
Weak if: always() assertion in new test
strings.Contains(output, "if: always()") is satisfied by the Print firewall logs step that precedes the upload step — it does not confirm the condition is on the upload step specifically. Tightening to a combined substring ("- name: Upload Firewall Logs\n if: always()") would make the test meaningful.
🔎 Code quality review by PR Code Quality Reviewer · 199.1 AIC · ⌖ 7.44 AIC · ⊞ 5.1K
Comments that could not be inline-anchored
pkg/workflow/engine_firewall_support.go:154
Latent duplicate-step hazard: defaultGetSquidLogsSteps (and the three engine GetSquidLogsSteps methods that delegate to it) is never called from any compiler code path — it is effectively dead code. This PR correctly adds the upload step directly in generateSummarySteps, but leaves the orphaned methods intact. If a future change wires GetSquidLogsSteps back into a compiler path alongside generateSummarySteps, every firewall-enabled workflow will emit two Upload Firewall Logs…
pkg/workflow/engine_firewall_support_test.go:318
Ambiguous assertion — passes for the wrong reason: strings.Contains(output, "if: always()") is satisfied by the Print firewall logs step that appears earlier in the same output. If a bug removed if: always() from only the Upload Firewall Logs step, this assertion would still pass.
<details>
<summary>💡 Suggested fix</summary>
Test a substring that ties the step name to the condition in one shot:
if !strings.Contains(output, "- name: Upload Firewall Logs\n if: alw…
</details>
<details><summary>pkg/workflow/compiler_yaml_main_job.go:693</summary>
**Double-upload of firewall logs on every firewall-enabled run**: `collectArtifactPaths` already adds `constants.AWFProxyLogsDir + "/"` to the unified `Upload agent artifacts` step (line 652 of the same file). This new `generateSquidLogsUploadStep` call emits the *same path* to a second `firewall-logs-{workflow}` artifact. The result is that every firewall-enabled workflow uploads the firewall logs directory **twice** on every run — to both the `agent` artifact and the new `firewall-logs-*` art…
</details>|
@copilot run pr-finisher skill |
In 2/19 sampled firewall-enabled runs, the squid
access.logwas absent from the artifact — both were earlyERR_CONFIGfailures, exactly the runs where egress logs matter most. The unified upload already runs withif: always(), but the firewall logs directory was empty/absent when AWF exited before writing proxy logs, soif-no-files-found: ignoresilently skipped it.Changes
generateFirewallLogParsingStep(engine_firewall_support.go)sudo mkdir -p .../firewall/logs .../firewall/auditbefore thechmod, ensuring the directories exist in the artifact even when AWF writes nothing to them (no proxy traffic on startup failure)generateSummarySteps(compiler_yaml_main_job.go)Upload Firewall Logsstep (if: always(), artifactfirewall-logs-{workflow}) for every firewall-enabled workflow — thegenerateSquidLogsUploadStepfunction already existed but was never called from the compileragentartifact, making them easy to locate on failed runsTests / golden fixtures
TestGenerateFirewallLogParsingStepFixesFirewallPermissionsto assertmkdir -pTestGenerateSummaryStepsIncludesFirewallLogsUploadasserts the dedicated upload step is emitted when firewall is enabled