Skip to content

refactor(control-plane): split handlers/nodes.go into 5 focused files#481

Open
kiranannadatha8 wants to merge 1 commit intoAgent-Field:mainfrom
kiranannadatha8:refactor/split-nodes-handler
Open

refactor(control-plane): split handlers/nodes.go into 5 focused files#481
kiranannadatha8 wants to merge 1 commit intoAgent-Field:mainfrom
kiranannadatha8:refactor/split-nodes-handler

Conversation

@kiranannadatha8
Copy link
Copy Markdown

Closes #416

Summary

Splits control-plane/internal/handlers/nodes.go (1753 LOC, 22 funcs, 3 types, 2 package vars — mixing registration, heartbeat, list/detail, status lifecycle, and URL-discovery helpers) into 5 focused files within the same handlers package. Pure file moves: no renames, no behavior change.

File layout

File LOC Contents
nodes_register.go 950 RegisterNodeHandler, RegisterServerlessAgentHandler, readCloser + validate, and all URL/candidate helpers (validateCallbackURL, extractPortFromURL, gatherCallbackCandidates, normalizeCandidate, resolveCallbackCandidates, normalizeServerlessDiscoveryURL, isServerlessDiscoveryHostAllowed)
nodes_heartbeat.go 298 HeartbeatHandler, HeartbeatCache, CachedNodeData, heartbeatCache/dbUpdateThreshold vars, shouldUpdateDatabase, processHeartbeatAsync
nodes_status.go 460 Lifecycle/status: UpdateLifecycleStatusHandler, GetNodeStatusHandler, RefreshNodeStatusHandler, BulkNodeStatusHandler, RefreshAllNodeStatusHandler, StartNodeHandler, StopNodeHandler
nodes_list.go 57 ListNodesHandler
nodes_detail.go 36 GetNodeHandler

Before: 1 file × 1753 LOC → After: 5 files summing to 1801 LOC (+48 from 4 extra package/import blocks + gofmt of a pre-existing stray indentation in the register section).

The old nodes.go is deleted. The issue allows nodes.go to remain for "tiny shared helpers only" — after the split there are no shared helpers left that would otherwise be homeless, so an empty nodes.go would be noise. Git detects the rename of nodes.gonodes_register.go (54% similarity), which keeps the diff reviewable.

Why this split

The issue proposed: nodes_register.go / nodes_heartbeat.go / nodes_list.go / nodes_detail.go / (nodes_tags.go if tag approval lives here) / nodes.go (shared helpers).

Tag approval handlers are in handlers/admin/tag_approval.go, not this file, so no nodes_tags.go. The 7 status/lifecycle handlers (UpdateLifecycleStatusHandler, GetNodeStatusHandler, RefreshNodeStatusHandler, BulkNodeStatusHandler, RefreshAllNodeStatusHandler, StartNodeHandler, StopNodeHandler) form a natural group and don't fit into detail/list, so they live together in nodes_status.go. Open to re-splitting if the reviewer prefers a different boundary.

Guardrails respected

  • No renames — diff <(grep '^(func|type|var|const) ' original | sort) <(cat new_files | grep '^(func|type|var|const) ' | sort) is empty
  • No behavior change — function bodies copied verbatim; gofmt'd per Go style
  • Existing tests unchanged — nodes_discovery_test.go, nodes_rest_test.go, nodes_test.go, nodes_serverless_revocation_test.go, and the coverage_*_test.go files that poke heartbeatCache/processHeartbeatAsync/readCloser/CachedNodeData by name all resolve unchanged via same-package visibility

Test plan

  • cd control-plane && go build ./...green
  • cd control-plane && go test ./internal/handlers/... -count=1ok across all 5 handler sub-packages (handlers, handlers/admin, handlers/agentic, handlers/connector, handlers/ui)
  • cd control-plane && go vet ./...clean
  • gofmt -l control-plane/internal/handlers/nodes*.goclean
  • cd control-plane && go test ./... -count=1 → handlers green; only pre-existing failures in internal/application, internal/core/services, internal/server (TestGenerateAgentFieldServerID, TestDevServicePortHelpers*, TestGenerateAgentFieldServerIDFallsBackWhenGetwdFails, TestStartAdminGRPCServerReturnsListenError). These all reproduce on a fresh clone of main — unrelated to this PR
  • CI green

AI assistance

Authored with AI assistance (Claude Code). Please apply the ai-assisted tag.

Splits the 1753 LOC handlers/nodes.go (22 funcs mixing registration,
heartbeat, listing, status lifecycle, and URL-discovery helpers) into
5 focused files within the same handlers package:

  nodes_register.go  (950 LOC) - RegisterNodeHandler,
                                 RegisterServerlessAgentHandler, readCloser,
                                 validate, and all URL/candidate helpers
  nodes_heartbeat.go (298 LOC) - HeartbeatHandler + HeartbeatCache +
                                 shouldUpdateDatabase + processHeartbeatAsync
  nodes_status.go    (460 LOC) - UpdateLifecycleStatusHandler,
                                 GetNodeStatusHandler, RefreshNodeStatusHandler,
                                 BulkNodeStatusHandler, RefreshAllNodeStatusHandler,
                                 StartNodeHandler, StopNodeHandler
  nodes_list.go       (57 LOC) - ListNodesHandler
  nodes_detail.go     (36 LOC) - GetNodeHandler

Pure file moves within the handlers package: no renames, no behavior
change. The old nodes.go is deleted (issue says "tiny shared helpers
only, if any" — no such helpers remain after the split, so keeping an
empty file would be noise).

All existing tests (nodes_test.go, nodes_rest_test.go,
nodes_discovery_test.go, nodes_serverless_revocation_test.go, and the
coverage_* tests that reference heartbeatCache/processHeartbeatAsync/
readCloser by name) resolve unchanged via same-package visibility.

Closes Agent-Field#416

Co-Authored-By: Claude <noreply@anthropic.com>
@kiranannadatha8 kiranannadatha8 force-pushed the refactor/split-nodes-handler branch from 15a7897 to dacefe5 Compare April 19, 2026 03:46
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.

[Refactor] Split control-plane/internal/handlers/nodes.go (1684 LOC)

1 participant