Skip to content

Show the CHIP server Node ID once the server has started - #136

Merged
rquidute merged 4 commits into
project-chip:v2.16-cli-developfrom
rquidute:fix/1193-cli-node-id-after-server-start
Oct 28, 2026
Merged

rquidute merged 4 commits into
project-chip:v2.16-cli-developfrom
rquidute:fix/1193-cli-node-id-after-server-start

Conversation

@rquidute

Copy link
Copy Markdown
Contributor

Summary

Replace the one-shot Node ID query at executing with a background watcher. It polls /chip-server/info every 2 s and prints the Node ID banner (and manual pairing code) whenever server_started is true and the ID differs from the last one shown. The watcher is cancelled when the connection closes.

Why

The Node ID is regenerated when the chip server starts during suite setup, after the run is already executing. The old query printed a stale ID, so commands run from the semi-automated user prompt with that ID failed. Requires the companion backend PR (server_started field).

Python-test runs never start the chip server, so they no longer print a bogus Node ID.

Notes

  • server_started was added to api_lib_autogen/models.py by hand; regenerate with ./scripts/generate_client.sh once the backend is up to confirm it matches.

Testing

  • pytest tests/test_run: 430 passed; new tests for unstarted server, first display, unchanged ID, changed ID and the polling loop.
  • Ran TC-WAKEONLAN-4.1 via CLI against both branches: the banner showed Node ID: 0x4de0d0b685c67b43, matching the ID used for pairing in the backend log, plus the manual pairing code.

Closes #1193 (Node ID part)

The Node ID is regenerated when the chip server starts during suite setup,
after the run is already executing, so the one-shot query at 'executing'
printed a stale ID. Watch the backend instead and display the Node ID (and
manual pairing code) whenever the server is started and the ID changes.
@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1776d205-bf66-4446-b679-76023f8c105f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
Walkthrough

The websocket now polls for CHIP server information every two seconds while a run is active. It displays pairing information only after the server starts and when its Node ID differs from the last displayed ID. The watcher stops when the run finishes and is cancelled when the websocket loop exits. Tests cover watcher startup, polling, server status, pairing details, and Node ID changes.

Sequence Diagram(s)

sequenceDiagram
  participant TestRunSocket
  participant CHIPServerAPI
  participant PairingAPI
  participant Terminal
  loop While the run is active
    TestRunSocket->>CHIPServerAPI: Poll server status
    CHIPServerAPI-->>TestRunSocket: Return startup status and Node ID
    opt Server started with a new Node ID
      TestRunSocket->>PairingAPI: Request pairing details
      PairingAPI-->>TestRunSocket: Return manual pairing code
      TestRunSocket->>Terminal: Display Node ID and pairing code
    end
  end
Loading

Suggested reviewers: antonio-amjr

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Moderate · previous review 8b6ab

This change shows the CHIP server Node ID once the server has started. If the backend does not return the new server_started field, the Node ID banner and pairing code never appear, which is the problem the PR is meant to fix. Confirm that the backend returns the field, or treat only an explicit False as not started, before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title states the main change: the CHIP server Node ID is now shown once the server has started. The changeset replaces the one-time query at executing with a watcher that polls /chip-server/info a…
Description check Passed The description explains the change: a background watcher polls /chip-server/info every 2 s and prints the Node ID banner and manual pairing code when the server has started and the ID changed. It giv…
Linked Issues check Passed For the Node ID objective in [#1193], TestRunSocket starts one watcher at executing, polls CHIP server info every 2 seconds, waits for server_started, displays a changed Node ID and manual pairi…
Out of Scope Changes check Passed The changed model field, websocket watcher, and websocket tests directly support the Node ID objective in [#1193]. The tests provide required automated coverage. No unrelated change is demonstrated by…
Full details: Title check

Explanation

The title states the main change: the CHIP server Node ID is now shown once the server has started. The changeset replaces the one-time query at executing with a watcher that polls /chip-server/info and displays the Node ID when server_started is true and the ID changed. The title matches this primary behavior and is specific enough for a teammate scanning history.

Full details: Description check

Explanation

The description explains the change: a background watcher polls /chip-server/info every 2 s and prints the Node ID banner and manual pairing code when the server has started and the ID changed. It gives the motivation (the Node ID regenerates during suite setup, so the old query showed a stale ID), notes the hand-added server_started field, and lists test results. It is directly related to the changeset.

Full details: Linked Issues check

Explanation

For the Node ID objective in [#1193], TestRunSocket starts one watcher at executing, polls CHIP server info every 2 seconds, waits for server_started, displays a changed Node ID and manual pairing code, and cancels the watcher when the websocket closes. ChipServerInfo includes server_started. Tests cover an unstarted server, first display, unchanged and changed IDs, polling, and watcher startup. The separate simulated commissioning failure in [#1193] is outside this PR's stated Node ID scope.

Full details: Out of Scope Changes check

Explanation

The changed model field, websocket watcher, and websocket tests directly support the Node ID objective in [#1193]. The tests provide required automated coverage. No unrelated change is demonstrated by the supplied change summary or reviewed file evidence.

Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 7 functions across 3 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mergify

mergify Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 093de92a-7a49-4c34-bbfb-054ce6380e29
Commits

Reviewing files that changed from the base of the PR and between f6fbc34 and 8b6ab09.

Files selected for processing: 3
  • tests/test_run/test_websocket_additional.py
  • th_cli/api_lib_autogen/models.py
  • th_cli/test_run/websocket.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Comment thread th_cli/api_lib_autogen/models.py
A backend that predates the server_started field omits it; defaulting to False
would suppress the Node ID banner entirely (#1193).
@rquidute
rquidute force-pushed the fix/1193-cli-node-id-after-server-start branch from aa60953 to 7c003b2 Compare October 28, 2026 14:28
@rquidute rquidute self-assigned this Oct 28, 2026
@rquidute
rquidute merged commit ba40a92 into project-chip:v2.16-cli-develop Oct 28, 2026
5 checks passed
@rquidute
rquidute deleted the fix/1193-cli-node-id-after-server-start branch October 28, 2026 17:01
rquidute added a commit that referenced this pull request Oct 28, 2026
* Show the CHIP server Node ID once the server has started (#1193)

The Node ID is regenerated when the chip server starts during suite setup,
after the run is already executing, so the one-shot query at 'executing'
printed a stale ID. Watch the backend instead and display the Node ID (and
manual pairing code) whenever the server is started and the ID changes.

* Treat only an explicit server_started=False as not started

A backend that predates the server_started field omits it; defaulting to False
would suppress the Node ID banner entirely (#1193).

* Re-generated api classes

* isort

(cherry picked from commit ba40a92)
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.

1 participant