Skip to content

fix(ci): run CodeQL on merge-queue commits - #2870

Merged
Daniel Meppiel (danielmeppiel) merged 3 commits into
mainfrom
danielmeppiel-issue-2866-fix
Sep 7, 2026
Merged

fix(ci): run CodeQL on merge-queue commits#2870
Daniel Meppiel (danielmeppiel) merged 3 commits into
mainfrom
danielmeppiel-issue-2866-fix

Conversation

@danielmeppiel

@danielmeppiel Daniel Meppiel (danielmeppiel) commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

fix(ci): run CodeQL on merge-queue commits

TL;DR

Run the existing Python and Actions CodeQL analyses on merge-queue commits as well as PRs and main pushes. Preserve analysis identities, findings, and protection rules. Resolve the overlap with #2869 by retaining its broader Windows regression coverage and keeping the contributor guidance from this branch.

Related: #2866. Its original test-harness fixes have already landed through #2869.

Warning

This fixes the missing merge-queue trigger, not the separate API upload <default> comparison problem. Both repository analyses were already uploaded for the original PR merge commit; GitHub's CodeQL summary identifies an additional base-branch API configuration. That producer still needs matching PR/queue results. No analyses or findings were deleted, and no ruleset was weakened.

Problem (WHY)

  • main uses a merge queue and requires CodeQL code scanning, but codeql.yml had no merge_group trigger. PR results belong to a different commit and cannot supply the queue commit's analysis.
  • The original PR's CodeQL summary explicitly reports 1 configuration not found under API upload, despite both normal language analyses completing. Adding or renaming normal categories would not reproduce that producer.
  • [!] fix(tests): restore Windows cache recency CI coverage #2869 merged overlapping Windows fixes while this PR was open; retaining our older test implementations would discard its stronger worker-depth and socket-capability coverage.

The regression procedure follows the Agent Skills validation loop: "do the work, run a validator (a script, a reference checklist, or a self-check), fix any issues, and repeat until validation passes."

Approach (WHAT)

  • Add merge_group / checks_requested scoped to main.
  • Retain the workflow path, analyze job ID, Python/Actions matrix, and unconditional uploads.
  • Keep the external producer and findings intact; document why an unrelated workflow rerun cannot replace them.

Implementation (HOW)

File Remaining change against main
.github/workflows/codeql.yml Add the missing queue trigger without changing analysis configuration identities.
tests/unit/test_security_workflow_contracts.py Require queue coverage, unchanged PR/push branch selection, both language jobs, and unconditional uploads.
docs/src/content/docs/contributing/development-guide.md Explain commit-specific CodeQL coverage and distinguish separate API-upload producers.
docs/src/content/docs/contributing/integration-testing.md Explain native socket capabilities and short fixture roots that retain worker depth.
tests/utils/isolated_apm_environment.py Comment explaining why asyncio requires native capability detection; implementation is main's version.

The cache-recency and socket regression implementations from #2869 are retained unchanged. No dependency, production behavior, or changelog changes.

Diagrams

The highlighted interaction supplies the queue commit to the existing analyses; external API uploads remain a separate responsibility.

sequenceDiagram
    participant E as GitHub event
    participant W as codeql.yml
    participant A as Python and Actions analyses
    participant S as Code scanning
    alt Pull request or main push
        E->>W: Existing trigger
    else Merge queue
        rect rgb(255, 247, 200)
            E->>W: NEW checks_requested trigger
        end
    end
    W->>A: Analyze event commit
    A->>S: Upload existing analysis configurations
    Note over S: Separate API-upload producer remains external
Loading

Trade-offs

  • Run both analyses again for each queue commit rather than reuse an earlier PR result: the queue combines changes into a distinct commit.
  • Keep the separate API-upload configuration and its findings instead of deleting results, fabricating SARIF, or relaxing code-scanning protection. Its missing PR comparison remains unresolved.
  • Prefer the already-merged, more comprehensive Windows fix over duplicate competing implementations.

Benefits

  1. Every matching merge-queue event starts both existing language analyses.
  2. Analysis keys and language configuration identity remain unchanged.
  3. The merged Windows worker-depth and network-denial regressions are preserved.

Validation

Local validation is against merged main 16edaf5ea. Remote CI is not claimed green.

Focused results and lint evidence

Before the workflow change, the new contract failed with KeyError: 'merge_group':

1 failed, 3 passed in 0.60s

After the fix and conflict resolution:

uv run --frozen --extra dev pytest -q \
  tests/unit/cache/test_git_cache_recency.py \
  tests/unit/test_isolated_network_guard.py \
  tests/unit/test_windows_compat_gate_workflow.py \
  tests/integration/test_isolated_apm_environment_contract.py \
  tests/unit/test_security_workflow_contracts.py --tb=short
80 passed in 24.62s

The canonical pre-push lint chain ran over src/, tests/, and the architecture scripts:

All checks passed!
1836 files already formatted
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)
[+] auth-signal lint clean
YAML I/O, file length, and portable path guards passed
[+] assertion-quality ratchet clean: AQ001=4, AQ002=12
[+] exact test duplicate ratchet clean: 1208 files, 0 allowed duplicate group(s)

The architecture-boundary command also exited successfully. The Mermaid diagram passed mmdc. Pytest emitted existing temporary-directory cleanup warnings after successful completion.

Scenario Evidence

# Scenario (user promise) Principle(s) Test(s) proving it Type
1 Enqueuing a PR schedules both required CodeQL analyses for the queue commit. Governed by policy tests/unit/test_security_workflow_contracts.py::test_codeql_covers_merge_queue_with_existing_analysis_configurations (regression trap) unit
2 Contributors can exercise real cache reuse and pruning at worker-equivalent path depth. OSS / community-driven tests/unit/cache/test_git_cache_recency.py (retained from #2869) integration
3 Hermetic Python imports preserve native socket capabilities and continue rejecting IP networking. Secure by default, OSS / community-driven tests/unit/test_isolated_network_guard.py; tests/integration/test_isolated_apm_environment_contract.py (retained from #2869) integration

How to test

  • Run the focused pytest command; expect all 80 cases to pass.
  • Once the workflow reaches main, enqueue a PR and confirm both CodeQL matrix jobs run on the queue commit.
  • If Code scanning still names API upload <default>, coordinate with that producer's owner; do not treat successful repository jobs as proof that the additional configuration ran.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Preserve native socket capabilities in the isolated network guard and shorten real sparse-cache fixture paths. Cover both failures in the Windows compatibility gate.

Refs #2866

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

Docs sync advisory

Verdict: no_change * Pages affected: 0 * LLM calls: 1/15

The deterministic L0 path gate confirms this PR only changes tests and existing contributor documentation. No production user surface changed, so no additional docs work or panel is needed. The orchestrator completed the classification directly after the classifier child lacked repository tools.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The new test_isolated_network_guard embeds an indented python -c script string that can raise IndentationError unless it is dedented before execution.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 High severity

New issues introduced by this change (1)
Severity Finding
High severity tests/​unit/​test_isolated_network_guard.py — The embedded Python script literal includes a leading space at the start of each line (e.g., the…
What changed in this PR

Repairs Windows release-smoke fixture failures by preserving native socket capability detection in the hermetic test harness and shortening sparse-cache fixture roots to stay under Windows path-length limits, plus adds Windows-gate regression coverage.

Changes:

  • Make the network-guarded socket sendmsg override conditional on native sendmsg availability to avoid triggering Unix-only feature paths on Windows.
  • Add/extend targeted tests to validate child-process imports under the network guard and to exercise sparse-cache recency scenarios within the windows_compat gate.
  • Document the harness behavior (native socket capability preservation) and the recommended short fixture-root pattern for sparse Git fixtures.
File Description
tests/​utils/​isolated_apm_environment.py Conditionally defines guarded sendmsg only when supported by the native socket implementation.
tests/​unit/​test_isolated_network_guard.py Adds regression tests covering optional socket-method preservation and child imports under the generated guard.
tests/​unit/​cache/​test_git_cache_recency.py Introduces a short recency_root fixture and promotes sparse recency scenarios to windows_compat.
tests/​integration/​test_isolated_apm_environment_contract.py Runs sendmsg network-denial probes only when the native socket provides sendmsg.
docs/​src/​content/​docs/​contributing/​integration-testing.md Documents the native socket capability preservation and short sparse-fixture roots for Windows path limits.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +49 to +50
"""
result = subprocess.run(
Preserve the worker-depth fixture and broader socket contracts from #2869, which supersede the overlapping local fix.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep existing analysis identities and security rules intact. Document the separate API-upload producer dependency without deleting its findings or claiming its PR comparison is resolved.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel Daniel Meppiel (danielmeppiel) changed the title fix(tests): repair Windows release smoke fixtures fix(ci): run CodeQL on merge-queue commits Sep 7, 2026
@danielmeppiel
Daniel Meppiel (danielmeppiel) merged commit d592b7b into main Sep 7, 2026
17 checks passed
@danielmeppiel
Daniel Meppiel (danielmeppiel) deleted the danielmeppiel-issue-2866-fix branch September 7, 2026 07:52
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.

2 participants