Skip to content

Commit ada7809

Browse files
committed
fix(client): use regex for host:port parsing to avoid bare IPv6 misparse
- Replace rsplit(":",1) with _HOST_PORT_RE regex in AsyncCoordinodeClient.__init__ - Add _cypher_param_name() to sanitise property keys before use as Cypher $params - Push ignore_rels filter into Cypher WHERE so LIMIT applies after filtering - Use uuid suffix for test node names to prevent collision on retry - Remove internal GAPS.md reference from xfail reason; set strict=True - Run uv sync before make proto (grpcio-tools must be installed first) - Pin all GitHub Actions refs to immutable commit SHAs - Verify gRPC port 7080 is up alongside HTTP :7084 in CI health wait Closes #1
1 parent 7f66257 commit ada7809

7 files changed

Lines changed: 76 additions & 42 deletions

File tree

.github/workflows/ci.yml

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ jobs:
1010
name: Lint
1111
runs-on: ubuntu-latest
1212
steps:
13-
- uses: actions/checkout@v4
13+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
1414

15-
- uses: astral-sh/setup-uv@v4
15+
- uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a # v4
1616
with:
1717
python-version: "3.11"
1818

@@ -28,11 +28,11 @@ jobs:
2828
matrix:
2929
python-version: ["3.11", "3.12", "3.13"]
3030
steps:
31-
- uses: actions/checkout@v4
31+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
3232
with:
3333
submodules: recursive
3434

35-
- uses: astral-sh/setup-uv@v4
35+
- uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a # v4
3636
with:
3737
python-version: ${{ matrix.python-version }}
3838

@@ -55,19 +55,20 @@ jobs:
5555
- 7080:7080
5656
- 7084:7084
5757
steps:
58-
- uses: actions/checkout@v4
58+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
5959
with:
6060
submodules: recursive
6161

62-
- uses: astral-sh/setup-uv@v4
62+
- uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a # v4
6363
with:
6464
python-version: "3.11"
6565

6666
- name: Wait for coordinode
6767
run: |
68-
echo "Waiting for coordinode health endpoint..."
68+
echo "Waiting for coordinode to be ready (HTTP :7084 + gRPC :7080)..."
6969
for i in $(seq 1 30); do
70-
if curl -sf http://localhost:7084/health >/dev/null 2>&1; then
70+
if curl -sf http://localhost:7084/health >/dev/null 2>&1 && \
71+
(echo > /dev/tcp/localhost/7080) 2>/dev/null; then
7172
echo "coordinode is ready (attempt $i)"
7273
exit 0
7374
fi

.github/workflows/release-please.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
release-please:
1414
runs-on: ubuntu-latest
1515
steps:
16-
- uses: googleapis/release-please-action@v4
16+
- uses: googleapis/release-please-action@16a9c90856f42705d54a6fda1823352bdc62cf38 # v4
1717
id: rp
1818
with:
1919
config-file: release-please-config.json

.github/workflows/release.yml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ jobs:
2727
- name: llama-index-graph-stores-coordinode
2828
path: llama-index-coordinode
2929
steps:
30-
- uses: actions/checkout@v4
30+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
3131
with:
3232
submodules: recursive
3333
fetch-depth: 0 # hatch-vcs needs full history for tag detection
3434

35-
- uses: astral-sh/setup-uv@v4
35+
- uses: astral-sh/setup-uv@38f3f104447c67c051c4a08e39b64a148898af3a # v4
3636
with:
3737
python-version: "3.11"
3838

@@ -48,7 +48,7 @@ jobs:
4848
run: uv run python -m build
4949

5050
- name: Upload dist artifact
51-
uses: actions/upload-artifact@v4
51+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
5252
with:
5353
name: dist-${{ matrix.package.name }}
5454
path: ${{ matrix.package.path }}/dist/
@@ -68,12 +68,12 @@ jobs:
6868
- langchain-coordinode
6969
- llama-index-graph-stores-coordinode
7070
steps:
71-
- uses: actions/download-artifact@v4
71+
- uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
7272
with:
7373
name: dist-${{ matrix.package }}
7474
path: dist/
7575

76-
- uses: pypa/gh-action-pypi-publish@release/v1
76+
- uses: pypa/gh-action-pypi-publish@cef221092ed1bacb1cc03d23a2d87d1d172e277b # release/v1
7777
with:
7878
packages-dir: dist/
7979

@@ -84,13 +84,13 @@ jobs:
8484
permissions:
8585
contents: write
8686
steps:
87-
- uses: actions/download-artifact@v4
87+
- uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
8888
with:
8989
pattern: dist-*
9090
merge-multiple: true
9191
path: dist/
9292

93-
- uses: softprops/action-gh-release@v2
93+
- uses: softprops/action-gh-release@153bb8e04406b158c6c84fc1615b65b24149a1fe # v2
9494
with:
9595
prerelease: ${{ contains(github.ref_name, 'a') || contains(github.ref_name, 'b') || contains(github.ref_name, 'rc') }}
9696
generate_release_notes: true

Makefile

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,18 @@ proto-check:
2727
@test -f $(PROTO_OUT)/coordinode/v1/query/cypher_pb2.py || \
2828
(echo "ERROR: Proto stubs not generated. Run: make proto" && exit 1)
2929

30-
# Install using uv (recommended for contributors)
31-
install: proto
30+
# Install using uv (recommended for contributors).
31+
# uv sync runs first — it installs grpcio-tools which proto generation requires.
32+
install:
3233
uv sync
34+
$(MAKE) proto
3335

3436
# Install using pip (alternative — works without uv)
35-
install-pip: proto
37+
install-pip:
3638
pip install -e "coordinode[dev]"
3739
pip install -e langchain-coordinode/
3840
pip install -e llama-index-coordinode/
41+
$(MAKE) proto
3942

4043
test: proto-check test-unit
4144

coordinode/client.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import asyncio
88
import logging
9+
import re
910
from collections.abc import Sequence
1011
from typing import Any
1112

@@ -22,6 +23,12 @@
2223

2324
logger = logging.getLogger(__name__)
2425

26+
# Matches "host:port" strings where host is either a bracketed IPv6 address
27+
# ([::1], [2001:db8::1]) or a name/IPv4 with no colons. Unbracketed IPv6
28+
# addresses (e.g. "2001:db8::1") are intentionally NOT matched — they cannot
29+
# be reliably distinguished from a "host:port" pair.
30+
_HOST_PORT_RE = re.compile(r"^(\[.+\]|[^:]+):(\d+)$")
31+
2532
# ── Low-level helpers ────────────────────────────────────────────────────────
2633

2734

@@ -105,13 +112,11 @@ def __init__(
105112
timeout: float = 30.0,
106113
) -> None:
107114
# Support "host:port" as a single string (common gRPC convention).
108-
# Parse whenever the last colon-delimited segment is numeric, regardless
109-
# of default port. IPv6 bracket notation ([::1]:7080) is handled correctly
110-
# by rsplit(":", 1): "[::1]" + "7080".
111-
if ":" in host:
112-
_h, _p = host.rsplit(":", 1)
113-
if _p.isdigit():
114-
host, port = _h, int(_p)
115+
# _HOST_PORT_RE matches "hostname:port" and "[IPv6]:port" but not bare
116+
# IPv6 addresses, avoiding the ambiguity of rsplit(":", 1) on "::1".
117+
m = _HOST_PORT_RE.match(host)
118+
if m:
119+
host, port = m.group(1), int(m.group(2))
115120
self._host = host
116121
self._port = port
117122
self._tls = tls

llama-index-coordinode/llama_index/graph_stores/coordinode/base.py

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,19 @@ def _cypher_ident(value: str) -> str:
2424
return f"`{value.replace('`', '``')}`"
2525

2626

27+
def _cypher_param_name(key: str) -> str:
28+
"""Return a valid Cypher parameter name derived from *key*.
29+
30+
Cypher parameter names must be valid identifiers (letters, digits, ``_``).
31+
Replace any other character with ``_`` and prepend ``p_`` when the result
32+
starts with a digit.
33+
"""
34+
safe = "".join(c if c.isalnum() or c == "_" else "_" for c in key)
35+
if safe and safe[0].isdigit():
36+
safe = f"p_{safe}"
37+
return safe or "p_"
38+
39+
2740
class CoordinodePropertyGraphStore(PropertyGraphStore):
2841
"""LlamaIndex ``PropertyGraphStore`` backed by CoordiNode.
2942
@@ -78,9 +91,15 @@ def get(
7891
node_id = str(row.get("_nid", ""))
7992
nodes.append(_node_result_to_labelled(node_id, node_data))
8093
elif properties:
81-
where_clauses = " AND ".join(f"n.{_cypher_ident(k)} = ${k}" for k in properties)
94+
# Build param mapping: safe Cypher param name → original value.
95+
# Property keys may contain hyphens or other chars invalid in
96+
# Cypher parameter names; _cypher_param_name() sanitises them.
97+
param_map = {_cypher_param_name(k): v for k, v in properties.items()}
98+
where_clauses = " AND ".join(
99+
f"n.{_cypher_ident(k)} = ${_cypher_param_name(k)}" for k in properties
100+
)
82101
cypher = f"MATCH (n) WHERE {where_clauses} RETURN n, n.id AS _nid LIMIT 1000"
83-
result = self._client.cypher(cypher, params=properties)
102+
result = self._client.cypher(cypher, params=param_map)
84103
for row in result:
85104
node_data = row.get("n", {})
86105
node_id = str(row.get("_nid", ""))
@@ -145,15 +164,23 @@ def get_rel_map(
145164
return []
146165

147166
node_ids = [n.id for n in graph_nodes]
148-
ignored = set(ignore_rels) if ignore_rels else set()
167+
ignored = list(ignore_rels) if ignore_rels else []
168+
169+
# Push ignore_rels filter into Cypher so LIMIT applies after filtering;
170+
# a Python-side filter after LIMIT would silently truncate valid results.
171+
params: dict[str, object] = {"ids": node_ids}
172+
ignore_clause = ""
173+
if ignored:
174+
ignore_clause = " AND NONE(rel IN r WHERE type(rel) IN $ignored_rels)"
175+
params["ignored_rels"] = ignored
149176

150177
cypher = (
151178
f"MATCH (n)-[r*1..{depth}]->(m) "
152-
f"WHERE id(n) IN $ids "
179+
f"WHERE id(n) IN $ids{ignore_clause} "
153180
f"RETURN n, r, m, id(n) AS _src_id, id(m) AS _dst_id "
154181
f"LIMIT {limit}"
155182
)
156-
result = self._client.cypher(cypher, params={"ids": node_ids})
183+
result = self._client.cypher(cypher, params=params)
157184

158185
triplets: list[list[LabelledNode]] = []
159186
for row in result:
@@ -163,10 +190,6 @@ def get_rel_map(
163190
dst_id = str(row.get("_dst_id", ""))
164191
# Variable-length path [r*1..N] returns a list of relationship dicts.
165192
rels = row.get("r", [])
166-
# Skip paths that contain any ignored relationship type.
167-
if ignored and isinstance(rels, list):
168-
if any(isinstance(r, dict) and r.get("type") in ignored for r in rels):
169-
continue
170193
if isinstance(rels, list) and rels:
171194
first_rel = rels[0]
172195
rel_label = first_rel.get("type", "RELATED") if isinstance(first_rel, dict) else str(first_rel)

tests/integration/test_basic.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"""
99

1010
import os
11+
import uuid
1112

1213
import pytest
1314

@@ -37,35 +38,36 @@ def test_cypher_return_literal(client):
3738
def test_create_and_get_node(client):
3839
# CREATE returns the node as an integer ID; RETURN n.prop verifies properties.
3940
# Note: id(n) is not yet implemented in alpha — use RETURN n.name.
41+
# UUID suffix prevents collisions when tests run in parallel or are retried.
42+
name = f"sdk-test-node-{uuid.uuid4().hex[:8]}"
4043
result = client.cypher(
4144
"CREATE (n:IntegrationTest {name: $name}) RETURN n.name AS name",
42-
params={"name": "sdk-test-node"},
45+
params={"name": name},
4346
)
4447
assert result, "CREATE returned no rows"
45-
assert result[0]["name"] == "sdk-test-node"
48+
assert result[0]["name"] == name
4649

4750
# Verify node is retrievable
4851
found = client.cypher(
4952
"MATCH (n:IntegrationTest {name: $name}) RETURN n.name AS name",
50-
params={"name": "sdk-test-node"},
53+
params={"name": name},
5154
)
5255
assert found, "MATCH returned no rows"
53-
assert found[0]["name"] == "sdk-test-node"
56+
assert found[0]["name"] == name
5457

5558
# Clean up
5659
client.cypher(
5760
"MATCH (n:IntegrationTest {name: $name}) DELETE n",
58-
params={"name": "sdk-test-node"},
61+
params={"name": name},
5962
)
6063

6164

6265
@pytest.mark.xfail(
6366
reason=(
6467
"VectorServiceImpl is a stub in server/src/services/vector.rs — always returns []."
6568
" HNSW algorithm (coordinode-vector crate) is implemented, but not wired to the RPC handler."
66-
" Tracked as gap G007 in coordinode-python GAPS.md."
6769
),
68-
strict=False,
70+
strict=True,
6971
)
7072
def test_vector_search(client):
7173
# Insert a node with an embedding, then search for it.

0 commit comments

Comments
 (0)