Repository navigation
MATTER-6231: Implement chip-tool skill - #1145
Michael Duggan (miduggan24) wants to merge 6 commits into
Conversation
|
bugbot run |
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit c264f7a. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Last node id never persisted
- Changed write_last_node_id and cmd_start_thread to use 'sh -c' instead of 'bash -lc' so the shell redirect is properly passed as part of the command string.
- ✅ Fixed: Check hides SSH auth failures
- Removed hub_is_valid call from require_host so cmd_check's auth_failed handling is now reachable and properly surfaces SSH key failures to users.
- ✅ Fixed: Dataset errors treated as hex
- Added error detection in read_dataset and cmd_dataset to check if ot-ctl output starts with 'Error' or contains error patterns, returning None/exit 1 instead of treating errors as valid hex datasets.
Or push these changes by commenting:
@cursor push 14aa892576
Preview (14aa892576)
diff --git a/.cursor/skills/chip-tool/scripts/hub.py b/.cursor/skills/chip-tool/scripts/hub.py
--- a/.cursor/skills/chip-tool/scripts/hub.py
+++ b/.cursor/skills/chip-tool/scripts/hub.py
@@ -122,21 +122,11 @@
def require_host() -> str:
- """Load host from JSON and verify reachability. Exit with setup help on failure."""
+ """Load host from JSON. Exit with setup help if missing."""
host = load_saved_host()
if not host:
print_user_setup("No Matter Hub host in ~/.silabs/matter-hub.json.")
sys.exit(1)
- if not hub_is_valid(host):
- print(
- f"Hub at {host} (from ~/.silabs/matter-hub.json) is unreachable or chip-tool is missing.",
- file=sys.stderr,
- )
- print(
- f"User must fix connectivity or update config: {HUB_SCRIPT} discover --host HOST",
- file=sys.stderr,
- )
- sys.exit(1)
return host
@@ -191,9 +181,14 @@
return 1
if proc.returncode != 0:
return print_proc(proc)
- line = (proc.stdout or "").strip().splitlines()
- if line:
- print(line[0].strip())
+ lines = (proc.stdout or "").strip().splitlines()
+ if not lines:
+ return 0
+ first = lines[0].strip()
+ if first.startswith("Error") or ":" in first.split()[0] if first.split() else False:
+ print("No Thread dataset. Run dataset or start-thread first.", file=sys.stderr)
+ return 1
+ print(first)
return 0
@@ -217,7 +212,7 @@
host = require_host()
proc = run_ssh(
host,
- ["bash", "-lc", START_THREAD_SCRIPT],
+ ["sh", "-c", START_THREAD_SCRIPT],
timeout=120,
)
if auth_failed(proc):
@@ -233,13 +228,18 @@
if proc.returncode != 0:
return None
lines = (proc.stdout or "").strip().splitlines()
- return lines[0].strip() if lines else None
+ if not lines:
+ return None
+ first = lines[0].strip()
+ if first.startswith("Error") or ":" in first.split()[0] if first.split() else False:
+ return None
+ return first
def write_last_node_id(host: str, node_id: int) -> None:
"""Remember commissioned node id on the hub for ``show-node-id`` / later ``run``."""
remote = f"echo {node_id} > {LAST_NODE_ID_REMOTE}"
- run_ssh(host, ["bash", "-lc", remote])
+ run_ssh(host, ["sh", "-c", remote])
def cmd_show_node_id(args: argparse.Namespace) -> int:You can send follow-ups to the cloud agent here.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: SSH failures misreported as missing chip-tool
- Added ssh_conn_failed() helper to detect SSH connection failures and updated cmd_discover, cmd_check, and cmd_show_node_id to report actual SSH errors instead of misdiagnosing them as missing chip-tool or missing node id.
Or push these changes by commenting:
@cursor push 7f3f5b09de
Preview (7f3f5b09de)
diff --git a/.cursor/skills/chip-tool/scripts/hub.py b/.cursor/skills/chip-tool/scripts/hub.py
--- a/.cursor/skills/chip-tool/scripts/hub.py
+++ b/.cursor/skills/chip-tool/scripts/hub.py
@@ -116,6 +116,18 @@
)
+def ssh_conn_failed(proc: subprocess.CompletedProcess[str]) -> bool:
+ """Check if SSH failed due to connection issues (timeout, refused, bad host)."""
+ stderr = proc.stderr or ""
+ return proc.returncode != 0 and (
+ "Connection timed out" in stderr
+ or "Connection refused" in stderr
+ or "Could not resolve hostname" in stderr
+ or "No route to host" in stderr
+ or "Network is unreachable" in stderr
+ )
+
+
def require_host() -> str:
"""Load host from JSON. Exit with setup help if config is missing."""
host = load_saved_host()
@@ -133,6 +145,9 @@
if auth_failed(proc):
print_user_setup("SSH key auth failed.")
return 1
+ if ssh_conn_failed(proc):
+ print(f"SSH connection to {host} failed.", file=sys.stderr)
+ return print_proc(proc) or 1
if proc.returncode != 0:
print(f"{host} is not a Matter Hub (chip-tool missing).", file=sys.stderr)
return 1
@@ -155,6 +170,9 @@
if auth_failed(proc):
print_user_setup("SSH key auth failed.")
return 1
+ if ssh_conn_failed(proc):
+ print(f"SSH connection to {host} failed.", file=sys.stderr)
+ return print_proc(proc) or 1
if proc.returncode != 0:
print(f"chip-tool not found at {CHIP_TOOL}", file=sys.stderr)
return 1
@@ -240,6 +258,9 @@
def cmd_show_node_id(args: argparse.Namespace) -> int:
host = require_host()
proc = run_ssh(host, ["cat", LAST_NODE_ID_REMOTE])
+ if ssh_conn_failed(proc):
+ print(f"SSH connection to {host} failed.", file=sys.stderr)
+ return print_proc(proc) or 1
if proc.returncode != 0:
print("No last node id on hub.", file=sys.stderr)
return 1You can send follow-ups to the cloud agent here.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Unquoted remote chip-tool arguments
- Modified chip_tool_argv to quote all arguments using shlex.quote and wrap the command in sh -c to prevent shell re-parsing of arguments with spaces or metacharacters.
- ✅ Fixed: start-thread reports false success
- Added set -e to START_THREAD_SCRIPT so the script exits immediately on any ot-ctl command failure instead of only returning the last command's exit code.
Or push these changes by commenting:
@cursor push 52e15841fa
Preview (52e15841fa)
diff --git a/.cursor/skills/chip-tool/scripts/hub.py b/.cursor/skills/chip-tool/scripts/hub.py
--- a/.cursor/skills/chip-tool/scripts/hub.py
+++ b/.cursor/skills/chip-tool/scripts/hub.py
@@ -206,6 +206,7 @@
START_THREAD_SCRIPT = """
+set -e
sudo -n ot-ctl factoryreset
sleep 3
sudo -n ot-ctl srp server disable
@@ -270,7 +271,8 @@
def chip_tool_argv(extra: Sequence[str]) -> List[str]:
"""Build remote argv for a single chip-tool invocation."""
- return [CHIP_TOOL, *extra]
+ quoted_args = [shlex.quote(arg) for arg in [CHIP_TOOL, *extra]]
+ return [f"sh -c {shlex.quote(' '.join(quoted_args))}"]
def cmd_commission_thread(args: argparse.Namespace) -> int:You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c264f7a. Configure here.
eb87a10 to
46d85ec
Compare
1 participant
Issue Link:
MATTER-6231
Description of Problem/Feature:
Implementation of agent skill to run chip-tool on user's raspberry pi over non-interactive ssh session
Description of Fix/Solution:
Add agent skill
slab-chip-tooland a supporting scripthub.pythat runchip-toolandot-ctlon the hub through non-interactive ssh session with hub address stored locally in~/.silabs/matter-hub.json(which is written by user during one-time setup throughdiscover --hostarg inhub.py)SKILL.md: Documents agent rules for chip-tool, outlines commissioning and chip-tool device control workflowhub.py: Helper script for agent to drive chip-tool through non-interactive ssh sessiondiscover- User writes hub IP with --host argcheck- Verify SSH key auth, chip-tool binary, andot-ctl statedataset- Print active Thread datasetstart-thread- Factory reset OTBR and bring up a new Thread networkcommission-thread- ble-thread commissioning flowrun- Forward args to chip-tool for device controlshow-node-id- Print last commissioned node idlogs- Tailotbr-agentjournalMatter over WiFi support will be added in a follow-up PR
Wrote files to new
.agents/directory which allows all agent harness to pick up the new skill for CLI devsTesting Done:
TODO Plan:
User flow setup flow works
hub.py dataset returns active dataset hex
commission + toggle
check fails when config/keys missing
Tested skill availability for Cursor, Github Copilot, and Codex