Skip to content

Clarify Copilot guidance for framework-only invariants vs user-reachable validation #69586

Description

@javiercn

Summary

Update .github/copilot-instructions.md to require classifying who can violate a precondition before changing an internal assertion into a runtime exception. Preserve framework-only assertions and their release behavior unless there is evidence that application-controlled input can reach the condition or an explicit contract calls for a runtime failure.

Motivation and goals

In #68961, commit 6c10ebb2a713b616cdc4d4afcccc3c7d6fdfef28 replaced a Debug.Assert in EndpointHtmlRenderer.InitializeStandardComponentServicesAsync with an InvalidOperationException, and added a test expecting the exception. This condition describes a framework-controlled call-order invariant: the first initialization call owns endpoint-specific data. The existing assertion catches a framework developer mistake during development; in release, a later call reused the cached task. The new throw instead introduces a runtime failure path for that invariant.

The cross-cutting guidance correctly asks for exceptions for invalid public/external input, but the Copilot instructions do not distinguish such validation from an invariant exclusively controlled by framework code. An unqualified preference for surfacing errors can therefore encourage this change. The goal is to make the ownership and reachability check explicit, not to discourage legitimate runtime validation.

In scope

  • Add a concise rule under Task Scope and Completion or a dedicated Assertions and validation section of .github/copilot-instructions.md: before adding a throw or replacing a Debug.Assert, trace the callers and identify whether application/user input can cause the violation, or whether it requires framework code to break its own call-order/state invariant.
  • For framework-only invariants, retain the development-time assertion and the existing release behavior by default. Do not introduce a new runtime throw or a test that codifies it solely to make the invariant fail loudly in release. If a runtime failure is intentional, explain the reachable scenario and why the behavior change is needed.
  • For public/API arguments, configuration, external input, and supported application-driven invalid states, keep runtime validation and actionable exceptions. internal visibility by itself does not prove that a condition is framework-only.
  • When reviewing a replacement assertion, compare Debug and Release behavior and use a test at a faithful caller boundary to establish reachability rather than treating a direct call to an internal helper with impossible arguments as proof of a user scenario.

Suggested instruction text:

Distinguish framework-controlled invariants from invalid states reachable through supported application/user input before adding an exception or replacing Debug.Assert. Trace the real callers and contract. For a condition that only framework code can violate, preserve the assertion and existing Release behavior unless a deliberate runtime behavior change is justified; do not add a throw merely to harden that internal path. Keep runtime exceptions for invalid public/external inputs and supported application-controlled states. A direct test invocation of an internal method does not, by itself, establish user reachability.

Out of scope

Risks / unknowns

An overbroad reading could suppress needed runtime checks in internal code reached from public APIs. The instruction must key off the source of the invalid state and supported call paths, not method accessibility. It should allow a deliberate exception when a concrete reachable scenario and intended behavior are documented.

Examples

In EndpointHtmlRenderer.InitializeStandardComponentServicesAsync, a second framework-owned initialization supplying endpoint-specific values should remain a Debug.Assert while returning the cached task in Release, absent evidence of a supported app-controlled route to that call. By contrast, an invalid argument supplied through a public entry point should still receive a precise runtime exception.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions