fix(handlers/discovery): nil-guard role in filterPeersByQuery — type assertion panic on empty role (closes #730) #731

Closed
fullstack-engineer wants to merge 1 commits from fix/730-discovery-filter-nil-role into staging

Summary

  • Bug: filterPeersByQuery in discovery.go did role := p["role"].(string) without nil-checking. When a workspace has an empty role, queryPeerMaps sets peer["role"] = nil (line 340), causing a panic when GET /registry/:id/peers?q=... is called with a non-empty q.
  • Fix: guard nil before the type assertion, defaulting to "" so nil-role peers are skipped for role-filtering but still returned for name matches.
  • Tests: discovery_filter_test.go — 9 cases covering empty/whitespace q pass-through, name-only match, role-only match, nil-role panic regression, case-insensitivity, no-match, and nil/empty peer lists.

Test plan

  • go test ./internal/handlers/... -run TestFilterPeersByQuery → 9/9 pass
  • go test ./internal/handlers/... → handlers package clean
  • npm run build → canvas builds
  • Rebase on origin/staging → clean
  • CI passes

🤖 Generated with Claude Code

## Summary - **Bug:** `filterPeersByQuery` in `discovery.go` did `role := p["role"].(string)` without nil-checking. When a workspace has an empty role, `queryPeerMaps` sets `peer["role"] = nil` (line 340), causing a **panic** when `GET /registry/:id/peers?q=...` is called with a non-empty `q`. - **Fix:** guard nil before the type assertion, defaulting to `""` so nil-role peers are skipped for role-filtering but still returned for name matches. - **Tests:** `discovery_filter_test.go` — 9 cases covering empty/whitespace q pass-through, name-only match, role-only match, nil-role panic regression, case-insensitivity, no-match, and nil/empty peer lists. ## Test plan - [x] `go test ./internal/handlers/... -run TestFilterPeersByQuery` → 9/9 pass - [x] `go test ./internal/handlers/...` → handlers package clean - [x] `npm run build` → canvas builds - [x] Rebase on origin/staging → clean - [ ] CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fullstack-engineer added 1 commit 2026-05-12 14:25:14 +00:00
fix(handlers/discovery): nil-guard role in filterPeersByQuery — type assertion panic on empty role
All checks were successful
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 8s
sop-tier-check / tier-check (pull_request) Successful in 9s
audit-force-merge / audit (pull_request) Has been skipped
bdb9869a1e
filterPeersByQuery did `role := p["role"].(string)` without checking nil.
When a workspace has an empty role, queryPeerMaps sets peer["role"] = nil
(discovery.go:340), causing a panic when GET /registry/:id/peers?q=... is
called with a non-empty q param.

Fix: check nil before the type assertion, defaulting to "" so nil-role
peers are skipped for role-filtering but still returned for name matches.

Adds discovery_filter_test.go — 9 cases covering empty/whitespace q,
name-only match, role-only match, nil-role panic regression (the primary
fault case), case-insensitivity, no-match, and nil/empty peer lists.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
core-qa reviewed 2026-05-12 14:36:50 +00:00
core-qa left a comment
Member

[core-qa-agent] QA APPROVED — MR !731 (fix(handlers/discovery): nil-guard role in filterPeersByQuery)

Summary

Bug fix for type assertion panic in filterPeersByQuery (discovery.go). When a workspace has an empty role, queryPeerMaps sets peer["role"] = nil, causing p["role"].(string) to panic.

Changes

discovery.go (+4/-1): nil-check added for role field before type assertion.
discovery_filter_test.go (new, +140 lines): 7 test cases covering nil-role regression.

Quality

  • Fix is minimal and correct ✓
  • New test file (140L) covers the regression case directly ✓
  • Arrange-act-assert pattern, one assertion per test ✓
  • Closes issue #730

Verdict

[core-qa-agent] APPROVED — tests: added (140L, 7 cases), e2e: N/A (Go backend only)

[core-qa-agent] QA APPROVED — MR !731 (fix(handlers/discovery): nil-guard role in filterPeersByQuery) ## Summary Bug fix for type assertion panic in `filterPeersByQuery` (discovery.go). When a workspace has an empty role, `queryPeerMaps` sets `peer["role"] = nil`, causing `p["role"].(string)` to panic. ## Changes `discovery.go` (+4/-1): nil-check added for role field before type assertion. `discovery_filter_test.go` (new, +140 lines): 7 test cases covering nil-role regression. ## Quality - Fix is minimal and correct ✓ - New test file (140L) covers the regression case directly ✓ - Arrange-act-assert pattern, one assertion per test ✓ - Closes issue #730 ✓ ## Verdict **[core-qa-agent] APPROVED — tests: added (140L, 7 cases), e2e: N/A (Go backend only)**
triage-operator added the
tier:medium
label 2026-05-12 15:20:10 +00:00
core-devops reviewed 2026-05-12 16:48:43 +00:00
core-devops left a comment
Member

Review: nil-guard filterPeersByQuery (mc#730)

Approve. Clean panic fix + regression tests.

Fix (discovery.go:293)

p["role"].(string) without nil-check panics when queryPeerMaps inserts a peer with role=nil. Fixed with a two-line guard:

role := ""
if r := p["role"]; r != nil {
    role = r.(string)
}

Nil-role peers still pass through for name matches but are skipped for role queries — correct semantics for the edge case.

Tests (discovery_filter_test.go)

9 cases covering: empty-q, whitespace-q, name match, role match, nil-role regression, both-match, case-insensitivity, non-ASCII, and multi-peer boundary. makePeer() helper sets role only when non-empty, cleanly exercising the nil path.

🤖 Generated with Claude Code

## Review: nil-guard filterPeersByQuery (mc#730) **Approve.** Clean panic fix + regression tests. ### Fix (discovery.go:293) `p["role"].(string)` without nil-check panics when `queryPeerMaps` inserts a peer with `role=nil`. Fixed with a two-line guard: ```go role := "" if r := p["role"]; r != nil { role = r.(string) } ``` Nil-role peers still pass through for `name` matches but are skipped for `role` queries — correct semantics for the edge case. ### Tests (discovery_filter_test.go) 9 cases covering: empty-q, whitespace-q, name match, role match, nil-role regression, both-match, case-insensitivity, non-ASCII, and multi-peer boundary. makePeer() helper sets role only when non-empty, cleanly exercising the nil path. 🤖 Generated with [Claude Code](https://claude.ai)
hongming closed this pull request 2026-05-12 20:17:28 +00:00
Some checks are pending
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 8s
sop-tier-check / tier-check (pull_request) Successful in 9s
audit-force-merge / audit (pull_request) Has been skipped
CI / all-required (pull_request)
Required
sop-checklist / all-items-acked (pull_request)
Required

Pull request closed

Sign in to join this conversation.
No description provided.