[/tdd] formalValidationBlocksEmit is a trivial validateErr != nil wrapper — it does not actually verify that lock-file emission is blocked in the production pipeline.

func formalValidationBlocksEmit(validateErr error) bool {
    return validateErr != nil
}

@copilot please address this.

[/tdd] P3 calls validateStrictNetwork without first setting compiler.strictMode = true, but the production implementation does not check strictMode — it always enforces the wildcard rule.

@copilot please address this.

[/tdd] validateNetworkAllowedDomains(nil) passes unconditionally in production but that is not tested against spec P3 — only the "nil is OK" assertion is made; there is no negative test for a non-nil invalid domain.

@copilot please address this.

[/tdd] P10 tests token isolation using hand-crafted in-memory maps — it does not call any production code that builds or validates the agent job environment.

@copilot please address this.

[/tdd] P8 duplicates the canonical job-name slice literal rather than deriving it from a shared authoritative source, meaning the test can drift from the real production ordering.

canonical := []string{
    string(constants.PreActivationJobName),
    ...
    string(constants.ConclusionJobName),
}
assert.True(t, formalJobOrderValid(canonical), ...)

@copilot please address this.

[/tdd] P2 skips PermissionMetadata in the loop but the production validator already excludes it — the skip comment is misleading about what the validator actually does.

@copilot please address this.

[/tdd] P7 tests the monotonicity of three compile-time constants — the property is trivially true by definition of the constant values and would never fail unless someone changed the constants themselves.

assert.True(t,
    int(formalConformanceLevelComplete) >= int(formalConformanceLevelStandard) &&
        int(formalConformanceLevelStandard) >= int(formalConformanceLevelBasic),
    "conformance level constants must be positive integers satisfying the ordering")

@copilot please address this.

[/tdd] P9 calls SetStrictMode(true) on a fresh compiler for the network test but the earlier dangerous-permissions half of P9 does not go through Compile() — the two sub-invariants test different code surfaces under different conditions.

// Dangerous permissions
perms := NewPermissions()
perms.Set(PermissionIssues, PermissionWrite)
err := validateDangerousPermissions(&WorkflowData{Permissions: "permissions: {}"}, perms)
require.Error(t, err, "dangerous permissions must be rejected at compile time")

// Strict-mode network
compiler := NewCompiler()
compiler.SetStrictMode(true)
strictErr := compiler.validateStrictNetwork(...)

@copilot please address this.

[/tdd] formalJobOrderValid silently returns true for any input slice that contains none of the canonical names, including an empty slice — this makes the helper useless as a regression guard for partial job lists.

func formalJobOrderValid(order []string) bool {
    // ...
    for i := 1; i < len(canonical); i++ {
        a, okA := idx[canonical[i-1]]
        b, okB := idx[canonical[i]]
        if okA && okB && a >= b {
            return false
        }
    }
    return true
}

@copilot please address this.

[/tdd] P5 does not cover the legacy top-level SandboxConfig.Type field path through isSandboxEnabled, leaving a supported code branch untested.

legacySandbox := &SandboxConfig{Type: SandboxTypeAWF}
assert.True(t, isSandboxEnabled(legacySandbox, nil),
    "legacy top-level sandbox type must still be recognised")

@copilot please address this.

P9 — SetStrictMode(true) does not affect validateStrictNetwork directly.

validateStrictNetwork does not read c.strictMode; the guard lives in the orchestrator validateStrictMode (strict_mode_validation.go:45). Calling compiler.SetStrictMode(true) and then immediately calling compiler.validateStrictNetwork(...) bypasses that guard — the test passes only because validateStrictNetwork always rejects "*" regardless of the strict-mode flag.

The test therefore does not prove the compile-gate predicate (P9); it just duplicates P3. Consider replacing with a call to c.validateStrictMode(frontmatter, &NetworkPermissions{Allowed: []string{"*"}}) to exercise the actual strict-mode path end-to-end.

@copilot please address this.

P10 — Token isolation is asserted on a hand-crafted map, not on compiled output.

The test constructs its own agentJobEnv and safeOutputsJobEnv maps and then checks that the trivial helper formalTokenAbsentFromEnv (which is just _, present := env[key]; return !present) returns the expected values. This is a closed-loop assertion: it can never fail because both the map and the assertion are controlled by the same test.

A real write would not be caught by this test — e.g. if the compiler were changed to accidentally inject GH_AW_APP_PRIVATE_KEY into the agent job env, this test would still pass because it never compiles a workflow and inspects the resulting job envs.

To meaningfully implement P10, compile a minimal workflow fixture, walk the emitted YAML agent-job env: block, and assert absence of the private-key secret key there.

@copilot please address this.

P6 — The false-branch assertion is tautologically true and adds no value.

formalValidationBlocksEmit is literally return err != nil. Asserting assert.False(t, formalValidationBlocksEmit(nil)) is identical to asserting assert.False(t, nil != nil) — it is always true by definition and cannot reveal a production defect.

Either remove this assertion to reduce noise, or add a case that would actually fail if the production validateDangerousPermissions incorrectly returned nil for a dangerous permission (which would also be caught by the first require.Error).

@copilot please address this.

P3 — The nil-network comment is misleading about which function is being tested.

The comment says "nil network permissions must not fail allowlist validation", but the code under test is validateNetworkAllowedDomains, not validateStrictNetwork. These two functions have opposite nil-handling:

• validateNetworkAllowedDomains(nil) returns nil (safe).
• validateStrictNetwork(nil) returns an error with "internal error: network permissions not initialized".

Adding a sub-test that calls compiler.validateStrictNetwork(nil) and asserts it does error would complete the nil-coverage picture and prevent a future regression if the nil guard in validateNetworkAllowedDomains is accidentally removed.

@copilot please address this.

P2 — Error message assertion may be fragile: check the actual message format.

assert.Contains(t, err.Error(), "write permissions") relies on the substring "write permissions" appearing in formatDangerousPermissionsError. The actual error text is "The agent job must not have write permissions." so the substring match holds today — but it is testing implementation prose rather than the semantic predicate.

A more robust pattern would be to use errors.As(err, ...)) or assert the error type, keeping the test decoupled from error message wording.

Also note: the skip comment for PermissionMetadata says it is "excluded from the rejection rule", but findWritePermissions skips it too — so setting meta(redacted) would not trigger an error in the first place. Consider adding an explicit sub-test that sets meta(redacted) and asserts NoError to document this intentional exception.

@copilot please address this.