Skip to content

Add features.awf-diagnostic-logs to enable AWF failure diagnostics artifact collection#26699

Merged
pelikhan merged 3 commits intomainfrom
copilot/add-diagnostic-logs-flag
Apr 16, 2026
Merged

Add features.awf-diagnostic-logs to enable AWF failure diagnostics artifact collection#26699
pelikhan merged 3 commits intomainfrom
copilot/add-diagnostic-logs-flag

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 16, 2026

AWF already had a --diagnostic-logs runtime flag, but workflows had no frontmatter-level control to enable it. This change wires that capability into features: so failed container startup cases can emit actionable Docker diagnostics into firewall-audit-logs/diagnostics.

  • Feature-flag plumbing

    • Added new typed feature flag constant: awf-diagnostic-logs.
    • Integrated it with existing feature-resolution flow (features: + env override behavior already in place).
  • AWF runtime argument generation

    • Updated AWF arg builder to append --diagnostic-logs only when:
      • features.awf-diagnostic-logs: true
    • Default behavior remains unchanged (no extra diagnostics unless explicitly enabled).
  • Coverage

    • Added focused unit tests validating:
      • flag is absent by default
      • flag is present when awf-diagnostic-logs is enabled
    • Extended feature-constant assertions to include the new flag.
  • Documentation

    • Added frontmatter reference docs for features.awf-diagnostic-logs.
    • Updated glossary feature list to include this flag and intent.
features:
  awf-diagnostic-logs: true

Copilot AI changed the title [WIP] Add diagnostic logs collection on Docker container failure Add features.awf-diagnostic-logs to enable AWF failure diagnostics artifact collection Apr 16, 2026
Copilot AI requested a review from pelikhan April 16, 2026 18:25
@pelikhan pelikhan marked this pull request as ready for review April 16, 2026 18:29
Copilot AI review requested due to automatic review settings April 16, 2026 18:29
@pelikhan pelikhan merged commit bcf0751 into main Apr 16, 2026
8 of 51 checks passed
@pelikhan pelikhan deleted the copilot/add-diagnostic-logs-flag branch April 16, 2026 18:29
@github-actions github-actions Bot mentioned this pull request Apr 16, 2026
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

Adds a new workflow frontmatter feature flag (features.awf-diagnostic-logs) that, when enabled, appends --diagnostic-logs to AWF runtime arguments so failed container startups can emit actionable Docker diagnostics into the firewall audit artifacts.

Changes:

  • Introduces a new typed feature flag constant for awf-diagnostic-logs and asserts it in constants tests.
  • Updates the AWF argument builder to conditionally append --diagnostic-logs when the feature is enabled.
  • Adds unit coverage for default/enabled behavior and documents the new frontmatter flag in reference docs and the glossary.
Show a summary per file
File Description
pkg/workflow/awf_helpers.go Conditionally appends --diagnostic-logs to AWF args based on the new feature flag.
pkg/workflow/awf_helpers_test.go Adds unit test coverage for presence/absence of --diagnostic-logs.
pkg/constants/feature_constants.go Defines the new awf-diagnostic-logs feature flag constant with usage docs.
pkg/constants/constants_test.go Extends feature-flag constant assertions to include the new flag.
docs/src/content/docs/reference/glossary.md Adds awf-diagnostic-logs to the list of common features: flags.
docs/src/content/docs/reference/frontmatter.md Documents features.awf-diagnostic-logs with intent and example usage.

Copilot's findings

Tip

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

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

Comment on lines +445 to +450
t.Run("does not include --diagnostic-logs when feature flag is absent", func(t *testing.T) {
config := AWFCommandConfig{
EngineName: "copilot",
WorkflowData: baseWorkflow(nil),
AllowedDomains: "github.com",
}
//
// features:
// awf-diagnostic-logs: true
AwfDiagnosticLogsFeatureFlag FeatureFlag = "awf-diagnostic-logs"
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 75/100

⚠️ Acceptable, with suggestions

Metric Value
New/modified tests analyzed 2
✅ Design tests (behavioral contracts) 2 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (50%)
Duplicate test clusters 0
Test inflation detected Yes (awf_helpers_test.go: +45 lines vs awf_helpers.go: +4 lines = 11.25:1)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Notes
TestFeatureFlagConstants (1 new table row) pkg/constants/constants_test.go:335 ✅ Design Verifies AwfDiagnosticLogsFeatureFlag == "awf-diagnostic-logs" behavioral contract
TestBuildAWFArgsDiagnosticLogs pkg/workflow/awf_helpers_test.go:431 ✅ Design Verifies BuildAWFArgs output with/without the feature flag — tests observable CLI args

Flagged Tests — Requires Review

No tests require review. All new tests are well-structured and enforce behavioral contracts.


Observations

Test Inflation (cosmetic, not a blocker)

awf_helpers_test.go adds 45 lines against only 4 lines in awf_helpers.go (11.25:1 ratio, exceeds 2:1 threshold). This triggers the technical inflation penalty, but in context the ratio is entirely justified:

  • The 4 production lines are a minimal conditional (if isFeatureEnabled(...) { append arg })
  • The 45 test lines include a reusable baseWorkflow builder closure and two well-structured subtests covering both the positive and negative conditions of the new flag
  • This is a classic pattern: simple conditional logic requires proportionally more test scaffolding

No action is required here.

Edge Case Coverage

TestBuildAWFArgsDiagnosticLogs covers both:

  • ✅ Positive: flag present → --diagnostic-logs included in args
  • ✅ Negative: flag absent → --diagnostic-logs NOT included in args

TestFeatureFlagConstants (the 1-line table row addition) is a constant-value contract test. No error path is applicable or expected for a string constant declaration.


Language Support

Tests analyzed:

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

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). Both tests verify behavioral contracts with descriptive assertion messages, proper build tags, and no mock library usage.


📖 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.

References: §24527131841

🧪 Test quality analysis by Test Quality Sentinel · ● 1.2M ·

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: 75/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). Both new tests verify behavioral contracts with proper build tags, descriptive assertion messages, and no mock library usage.

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.

feat: collect Docker operational logs on failure for AWF diagnostics

3 participants