fix: update TestFirewallArgsIntegration for --allow-domains→config file migration and strict mode id requirement

[Copilot]

TestFirewallArgsIntegration was failing in CI due to two separate behavioural changes that landed without updating the integration tests.

Changes

• --allow-domains moved to JSON config file: BuildAWFConfigJSON now writes network egress rules as "allowDomains" in a structured JSON config rather than passing --allow-domains as a CLI flag. Two test assertions were still checking for the old CLI flag string:

  // Before (broken)
  strings.Contains(lockYAML, "--allow-domains")
  // After
  strings.Contains(lockYAML, "allowDomains")

• Strict mode requires explicit sandbox.agent.id: validateStrictSandboxCustomization now rejects a sandbox.agent block with no id field in strict mode (the default). The workflow_pinning_old_AWF_version_does_not_emit_--exclude-env test workflow used sandbox.agent.version alone, causing compilation to fail before the assertion was ever reached. Added id: awf to the fixture:

  sandbox:
    agent:
      id: awf
      version: v0.25.0

[Copilot]

Pull request overview

Updates TestFirewallArgsIntegration to match recent compiler/AWF behavior changes so CI integration tests pass again.

Changes:

• Updates assertions to check for allowDomains in the embedded AWF config JSON instead of the legacy --allow-domains CLI flag.
• Updates the “old AWF version” workflow fixture to include sandbox.agent.id to satisfy strict-mode validation.

Show a summary per file

File	Description
pkg/workflow/firewall_args_integration_test.go	Aligns integration test expectations/fixtures with the config-file migration and strict-mode sandbox.agent.id requirement.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

pkg/workflow/firewall_args_integration_test.go:156

• Same as above: consider asserting on the quoted JSON key "allowDomains" instead of the unquoted substring to match existing integration-test patterns (e.g., pkg/workflow/blocked_domains_integration_test.go:83) and avoid accidental matches.

		if !strings.Contains(lockYAML, "allowDomains") {
			t.Error("Compiled workflow should contain 'allowDomains' in the AWF config JSON")

• Files reviewed: 1/1 changed files
• Comments generated: 1

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 100/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	3 (100%)
Duplicate test clusters	0
Test inflation detected	No
🚨 Coding-guideline violations	None

---

Test Classification Details

Test	File	Classification	Issues Detected
workflow with custom firewall args compiles correctly	pkg/workflow/firewall_args_integration_test.go:17	✅ Design	None
workflow without custom args uses only default flags	pkg/workflow/firewall_args_integration_test.go:92	✅ Design	None
workflow pinning old AWF version does not emit --exclude-env	pkg/workflow/firewall_args_integration_test.go:289	✅ Design	None

---

What Changed

This PR makes three targeted corrections to an existing integration test file:

1. --allow-domains → allowDomains (two sub-tests): The firewall domain configuration migrated from a CLI flag to a JSON config key. The assertions now correctly verify the compiled YAML contains allowDomains in the AWF config JSON rather than a --allow-domains CLI flag.

2. id: awf added to sandbox config (one sub-test): The strict mode now requires an explicit id: awf field. The test fixture was updated to reflect this requirement, ensuring the test exercises the pinned-version code path correctly.

All three changes update tests to match actual production behavior — this is exactly the right kind of maintenance work.

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All modified tests verify observable compiled output, cover error paths via t.Fatalf/t.Error, and include descriptive assertion messages. Build tag //go:build integration is correctly present. No mock libraries detected.

---

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

---

Language Support

Tests analyzed:

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

---

References: §25250703916

🧪 Test quality analysis by Test Quality Sentinel · ● 392.9K · ◷