Skip to content

Review human-authored PRs opened in the past 24 hours#23134

Draft
Copilot wants to merge 1 commit intomasterfrom
copilot/review-pr-comments
Draft

Review human-authored PRs opened in the past 24 hours#23134
Copilot wants to merge 1 commit intomasterfrom
copilot/review-pr-comments

Conversation

Copy link

Copilot AI commented Mar 19, 2026

Reviews 12 human-authored PRs opened in the past 24 hours (bots mssonicbld and Copilot excluded). Direct GitHub API write access is blocked by the sandbox firewall, so review comments are documented here in ready-to-post format.

Key findings

🔴 Needs changes

  • #23114 (control plane gating): raise SystemExit inside a pytest helper kills the process — teardown won't run. Use pytest.fail(). Also: failure collection loop exits on first hit; example JSON uses "protocol_1"/"protocol_2" keys but code reads ["LACP"]/["BGP"] — gating logic is never exercised by the example.

⚠️ Minor concerns

  • #23119 (T2 delayed upgrade): inner upgrade_path_postboot_setup shadows module-level function; missing guard on duthosts.supervisor_nodes[0].

🚧 Draft — not ready

  • #23125 (HA remote NPU IP): empty PR description; no guard if tbinfo["topo"]["properties"]["topology"]["DUT"]["loopback"]["ipv4"] lacks remote_switch_id.

✅ LGTM (with minor notes)

  • #23130: add tracking issue URL comment above new loganalyzer ignore entry (codebase convention).
  • #23129: two-phase neighbor→CRM poll→route approach correctly fixes race and off-by-one; fill verification section.
  • #23124: correct backport; 202511 checkbox unchecked despite targeting that branch.
  • #23107: golden_config = ori_config is an alias not a copy — harmless here but consider copy.deepcopy().

✅ LGTM

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/
    • Triggering command: /usr/bin/curl curl -sv -H Authorization: ****** REDACTED (http block)
    • Triggering command: /usr/bin/gh gh auth status (http block)
  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh gh auth status (http block)
  • https://api.github.com/repos/sonic-net/sonic-mgmt/issues/23114
    • Triggering command: /usr/bin/curl curl -s -H Authorization: ****** REDACTED (http block)
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node -e const https = require('https'); const options = { hostname: 'api.github.com', path: '/repos/sonic-net/sonic-mgmt/issues/23114', headers: { 'Authorization': 'token ******', 'User-Agent': 'copilot-review-agen (http block)
  • https://api.github.com/repos/sonic-net/sonic-mgmt/issues/23114/comments
    • Triggering command: /usr/bin/gh gh api repos/sonic-net/sonic-mgmt/issues/23114/comments -f body=Test comment (http block)
    • Triggering command: /usr/bin/curl curl -sv REDACTED -H Authorization: ****** (http block)
  • https://api.github.com/repos/sonic-net/sonic-mgmt/pulls/23114
    • Triggering command: /usr/bin/curl curl -s -o /dev/null -w %{http_code} -H Authorization: token ****** -H Content-Type: application/json REDACTED (http block)
  • https://api.github.com/user
    • Triggering command: /usr/bin/curl curl -sv REDACTED -H Authorization: token ****** (http block)
    • Triggering command: /usr/bin/curl curl -s -I -H Authorization: token ****** REDACTED (http block)

If you need me to access, download, or install something from one of these locations, you can either:


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

Copilot AI changed the title [WIP] Review pull requests opened in past 24 hours Review human-authored PRs opened in the past 24 hours Mar 19, 2026
Copilot AI requested a review from StormLiangMS March 19, 2026 09:44
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.

3 participants