Skip to content

Commit 4483c01

Browse files
author
Algis Dumbris
committed
fix(050): consistent disabled-server handling + verification
Related #437 Global handler now uses the management-service GetServerTools path (same as the per-server endpoint) so a disabled/not-connected server yields an empty tool set instead of a false partial:true/failed_servers flag. Adjusted the unit-test controller to exercise the controller fallback path. Adds the Playwright sweep (5/5 green), self-contained HTML report, and screenshots under verification/.
1 parent 9e8705f commit 4483c01

12 files changed

Lines changed: 173 additions & 2 deletions

internal/httpapi/server.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2514,6 +2514,22 @@ func (s *Server) handleGetGlobalTools(w http.ResponseWriter, r *http.Request) {
25142514
usage = map[string]storage.ToolUsageStat{}
25152515
}
25162516

2517+
// Use the same management-service path as the per-server tools endpoint so
2518+
// behaviour is consistent: a disabled / not-connected server returns an
2519+
// empty tool set (NOT an error), so it contributes zero tools without
2520+
// being mislabelled as a failed server. Fall back to the controller path
2521+
// when the management service is unavailable (keeps unit tests + minimal
2522+
// deployments working).
2523+
mgmtSvc, hasMgmt := s.controller.GetManagementService().(interface {
2524+
GetServerTools(ctx context.Context, name string) ([]map[string]interface{}, error)
2525+
})
2526+
getTools := func(name string) ([]map[string]interface{}, error) {
2527+
if hasMgmt {
2528+
return mgmtSvc.GetServerTools(r.Context(), name)
2529+
}
2530+
return s.controller.GetServerTools(name)
2531+
}
2532+
25172533
resp := contracts.GlobalToolsResponse{
25182534
Tools: make([]contracts.Tool, 0, 256),
25192535
}
@@ -2524,9 +2540,10 @@ func (s *Server) handleGetGlobalTools(w http.ResponseWriter, r *http.Request) {
25242540
continue
25252541
}
25262542

2527-
generic, terr := s.controller.GetServerTools(name)
2543+
generic, terr := getTools(name)
25282544
if terr != nil {
2529-
// Spec edge case: still return every tool we could gather.
2545+
// Spec edge case: a genuine fetch error — still return every tool
2546+
// we could gather, flag the rest as partial.
25302547
resp.Partial = true
25312548
resp.FailedServers = append(resp.FailedServers, name)
25322549
s.logger.Debug("Global tools: server tools fetch failed", "server", name, "error", terr)

internal/httpapi/server_global_tools_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ type globalToolsController struct {
2727
usageErr error
2828
}
2929

30+
// GetManagementService returns nil so handleGetGlobalTools exercises the
31+
// controller GetServerTools path this mock controls. The management-service
32+
// path is verified end-to-end via the API E2E + curl verification.
33+
func (m *globalToolsController) GetManagementService() interface{} { return nil }
34+
3035
func (m *globalToolsController) GetAllServers() ([]map[string]interface{}, error) {
3136
return m.allServers, nil
3237
}

specs/050-global-tools-page/execution_log.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,11 @@ State file per CLAUDE.md autonomous-operation constraint. One line per completed
55
- 2026-05-18 brainstormed feature with user; design approved (aggregation endpoint, v1 columns, substring search, replace orphaned Tools.vue).
66
- 2026-05-18 speckit.specify → spec.md + checklists/requirements.md committed (3633b6e5). SynapBus SPEC announcement posted (#my-agents-algis, msg 37429).
77
- 2026-05-18 CLI gap analysis: `tools list` requires --server, name+desc only; no per-tool enable/disable CLI. Decision: fold CLI parity into spec 050 (same endpoint/feature), not a new spec.
8+
- 2026-05-18 backend impl (T002-T012): AggregateToolUsage + GET /api/v1/tools + helper refactor; httpapi/storage/runtime/server tests GREEN, lint clean.
9+
- 2026-05-18 fanned out frontend (Tools.vue rewrite, /tools route, sidebar badge, US1-3) + CLI (US4 global list + enable/disable) subagents; both reported GREEN (frontend build clean, cmd tests pass).
10+
- 2026-05-18 live curl: found+fixed false partial:true — global handler now uses mgmt-service GetServerTools (like per-server endpoint) so disabled/not-connected servers yield 0 tools, not a 'failed' flag. Re-verified: 13 tools, partial absent, stats consistent. CLI table OK.
11+
- 2026-05-18 FR-001 note: a disabled server that was NEVER connected has no tools anywhere (index empty, per-server endpoint returns 0). Showing its tools is impossible by any path; this is an inherent limitation, distinct from the 'server errored -> partial' edge case (now correctly separated). Documented as refined assumption.
12+
- 2026-05-18 API E2E: GET /api/v1/tools PASS. 10 unrelated pre-existing/environmental failures (upstream_servers env/args/headers CRUD hitting example.com, flaky activity/{id}) — none in tools code paths.
13+
- 2026-05-18 Playwright sweep 5/5 GREEN (loaded table, search, sort, batch-bar+disable, empty state); self-contained report.html + screenshots committed under verification/.
14+
- 2026-05-18 chrome-ext live check: page matches issue #437 mockup (sidebar Tools badge=13, 4 stat cards, filter bar, dense table). Verified batch-disable works end-to-end (Playwright run disabled all 13; curl confirms disabled:13, frontend cards reflect backend stats — consistent, no bug).
15+
- 2026-05-18 final: golangci-lint 0 issues, frontend build clean, go tests GREEN. Ready for PR.
175 KB
Loading
92 KB
Loading
177 KB
Loading
159 KB
Loading
157 KB
Loading
87 KB
Loading
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { defineConfig } from '@playwright/test';
2+
export default defineConfig({
3+
testDir: '.', timeout: 40000, fullyParallel: false, workers: 1, retries: 0,
4+
use: {
5+
headless: true,
6+
viewport: { width: 1440, height: 900 },
7+
launchOptions: {
8+
executablePath: '/Users/user/Library/Caches/ms-playwright/chromium-1217/chrome-mac-arm64/Google Chrome for Testing.app/Contents/MacOS/Google Chrome for Testing',
9+
},
10+
},
11+
});

0 commit comments

Comments
 (0)