Clarify typed service sizing mappings

ktsaou · ktsaou · commit 7d892cdef977 · 2026-06-04T15:33:08.000+03:00
diff --git a/.agents/sow/current/SOW-0014-20260603-maintainability-hotspots.md b/.agents/sow/current/SOW-0014-20260603-maintainability-hotspots.md
@@ -827,17 +827,47 @@ Implementation validation:
   - Windows/MSYS CTest service slice passed: `test_win_service`, `test_win_service_extra`, and `test_win_service_payload_limits`.
   - `git diff --check` passed.
   - `bash .agents/sow/audit.sh` passed.
+
+### 2026-06-04 GitHub AI Quality Findings Review
+
+- The GitHub quality AI findings page reported seven findings across typed Go service wrappers and C Windows service code.
+- Response-batch mapping findings:
+  - Reported files include `src/go/pkg/netipc/service/apps_lookup/client.go`, `src/go/pkg/netipc/service/cgroups/client.go`, `src/go/pkg/netipc/service/cgroups_lookup/client.go`, `src/go/pkg/netipc/service/cgroups_lookup/client_windows.go`, and `src/libnetdata/netipc/src/service/netipc_service_win.c`.
+  - Same-failure search also found the equivalent Go Windows apps lookup wrapper and POSIX C service mapping.
+  - Decision: reject these as false positives, but clarify the code.
+  - Evidence: public typed Go configs expose `MaxRequestBatchItems` and `MaxResponsePayloadBytes`, but no `MaxResponseBatchItems`; C `nipc_client_config_t` and `nipc_server_config_t` likewise expose only `max_request_batch_items` and `max_response_payload_bytes`.
+  - Evidence: `docs/level1-wire-envelope.md` says `max_response_batch_items` is kept for wire symmetry and the agreed response batch count must equal the agreed request batch count.
+  - Implementation plan: keep behavior unchanged, but route response-batch assignments through private helpers named as typed response-batch symmetry helpers so future reviewers do not interpret the mapping as accidental copy-paste.
+- Lookup request-payload default finding:
+  - Reported file: `src/libnetdata/netipc/src/service/netipc_service_win.c`.
+  - Same-failure search found the equivalent POSIX C lookup server initializers.
+  - Decision: reject as intentional behavior, but clarify the code.
+  - Evidence: `docs/level2-typed-api.md` says typed callers do not provide `max_request_payload_bytes`; the library derives the initial proposal internally from method schema, batch size, and dynamic-field assumptions.
+  - Evidence: lookup requests contain dynamic path/PID arrays, and C lookup servers pre-size accepted-session SHM resources from `learned_request_payload_bytes` before serving typed requests.
+  - Risk of applying the suggested change: changing lookup server request defaults from the larger lookup default to the generic request default could add avoidable reconnect churn or undersized SHM setup for lookup workloads.
+  - Implementation plan: keep behavior unchanged, but replace direct `cgroups_response_payload_default()` calls in lookup request-default sites with a private helper named as a lookup request-payload default.
+  - Implemented:
+    - C POSIX and Windows typed service config conversion now routes response-batch symmetry through `nipc_service_common_typed_response_batch_items()`.
+    - C POSIX and Windows lookup server request defaults now route through `nipc_service_common_lookup_request_payload_default()`.
+    - Go POSIX and Windows typed service wrappers for apps lookup, cgroups lookup, and cgroups snapshot now route response-batch symmetry through per-package `typedResponseBatchItems()` helpers.
+  - Same-failure verification: no remaining direct `MaxResponseBatchItems: config.MaxRequestBatchItems` or `max_response_batch_items = config->max_request_batch_items` assignments in typed service wrappers.
+  - `cmake --build build` passed.
+  - `/usr/bin/ctest --test-dir build --output-on-failure -R 'service|cache'` passed: 13/13 focused service/cache tests.
+  - POSIX Go typed-service package tests passed for apps lookup, cgroups lookup, and cgroups snapshot.
+  - Windows Go typed-service package compilation/tests passed for apps lookup, cgroups lookup, and cgroups snapshot.
+  - Windows/MSYS rebuild passed for `netipc_service_win`, `test_win_service`, `test_win_service_extra`, `test_win_service_payload_limits`, `interop_service_win_c`, and `interop_cache_win_c`.
+  - Windows/MSYS CTest service slice passed: `test_win_service`, `test_win_service_extra`, and `test_win_service_payload_limits`.
   - `codacy-analysis analyze --output-format json` passed:
     - Checkov: 0 issues.
     - Opengrep/Semgrep: 0 issues.
     - Trivy: 0 issues.
     - cppcheck: 0 issues.
     - ShellCheck: 0 issues.
     - Spectral: 0 issues.
-    - total: 0 issues, 0 errors.
+  - total: 0 issues, 0 errors.
   - `bash .agents/sow/audit.sh` passed and reported SOW initialization complete and clean.
   - `git diff --check` passed.
-  - Sensitive-data scan across the touched SOW, C service, C service common, C codec, and CMake files returned no matches.
+  - Sensitive-data scan across the touched SOW, Go service wrappers, and C service files returned no matches.
 
 Sensitive data gate:
 
diff --git a/src/go/pkg/netipc/service/apps_lookup/client.go b/src/go/pkg/netipc/service/apps_lookup/client.go
@@ -12,7 +12,7 @@ func clientConfigToTransport(config ClientConfig) posix.ClientConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
@@ -23,7 +23,7 @@ func serverConfigToTransport(config ServerConfig) posix.ServerConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
diff --git a/src/go/pkg/netipc/service/apps_lookup/client_common.go b/src/go/pkg/netipc/service/apps_lookup/client_common.go
@@ -9,6 +9,11 @@ type Client struct {
 	inner *raw.Client
 }
 
+// typedResponseBatchItems keeps typed request/response batch counts symmetric.
+func typedResponseBatchItems(maxRequestBatchItems uint32) uint32 {
+	return maxRequestBatchItems
+}
+
 func NewClient(runDir, serviceName string, config ClientConfig) *Client {
 	return &Client{inner: raw.NewAppsLookupClient(runDir, serviceName, clientConfigToTransport(config))}
 }
diff --git a/src/go/pkg/netipc/service/apps_lookup/client_windows.go b/src/go/pkg/netipc/service/apps_lookup/client_windows.go
@@ -12,7 +12,7 @@ func clientConfigToTransport(config ClientConfig) windows.ClientConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
@@ -23,7 +23,7 @@ func serverConfigToTransport(config ServerConfig) windows.ServerConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
diff --git a/src/go/pkg/netipc/service/cgroups/client.go b/src/go/pkg/netipc/service/cgroups/client.go
@@ -12,7 +12,7 @@ func clientConfigToTransport(config ClientConfig) posix.ClientConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
@@ -23,7 +23,7 @@ func serverConfigToTransport(config ServerConfig) posix.ServerConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
diff --git a/src/go/pkg/netipc/service/cgroups/client_common.go b/src/go/pkg/netipc/service/cgroups/client_common.go
@@ -9,6 +9,11 @@ func snapshotDispatch(handler Handler) raw.DispatchHandler {
 	return raw.SnapshotDispatch(handler.Handle, handler.SnapshotMaxItems)
 }
 
+// typedResponseBatchItems keeps typed request/response batch counts symmetric.
+func typedResponseBatchItems(maxRequestBatchItems uint32) uint32 {
+	return maxRequestBatchItems
+}
+
 // Client is the public L2 client context for the cgroups-snapshot service.
 type Client struct {
 	inner *raw.Client
diff --git a/src/go/pkg/netipc/service/cgroups/client_windows.go b/src/go/pkg/netipc/service/cgroups/client_windows.go
@@ -12,7 +12,7 @@ func clientConfigToTransport(config ClientConfig) windows.ClientConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
@@ -23,7 +23,7 @@ func serverConfigToTransport(config ServerConfig) windows.ServerConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
diff --git a/src/go/pkg/netipc/service/cgroups_lookup/client.go b/src/go/pkg/netipc/service/cgroups_lookup/client.go
@@ -12,7 +12,7 @@ func clientConfigToTransport(config ClientConfig) posix.ClientConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
@@ -23,7 +23,7 @@ func serverConfigToTransport(config ServerConfig) posix.ServerConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
diff --git a/src/go/pkg/netipc/service/cgroups_lookup/client_common.go b/src/go/pkg/netipc/service/cgroups_lookup/client_common.go
@@ -9,6 +9,11 @@ type Client struct {
 	inner *raw.Client
 }
 
+// typedResponseBatchItems keeps typed request/response batch counts symmetric.
+func typedResponseBatchItems(maxRequestBatchItems uint32) uint32 {
+	return maxRequestBatchItems
+}
+
 func NewClient(runDir, serviceName string, config ClientConfig) *Client {
 	return &Client{inner: raw.NewCgroupsLookupClient(runDir, serviceName, clientConfigToTransport(config))}
 }
diff --git a/src/go/pkg/netipc/service/cgroups_lookup/client_windows.go b/src/go/pkg/netipc/service/cgroups_lookup/client_windows.go
@@ -12,7 +12,7 @@ func clientConfigToTransport(config ClientConfig) windows.ClientConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
@@ -23,7 +23,7 @@ func serverConfigToTransport(config ServerConfig) windows.ServerConfig {
 		PreferredProfiles:       config.PreferredProfiles,
 		MaxRequestBatchItems:    config.MaxRequestBatchItems,
 		MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,
-		MaxResponseBatchItems:   config.MaxRequestBatchItems,
+		MaxResponseBatchItems:   typedResponseBatchItems(config.MaxRequestBatchItems),
 		AuthToken:               config.AuthToken,
 	}
 }
diff --git a/src/libnetdata/netipc/src/service/netipc_service.c b/src/libnetdata/netipc/src/service/netipc_service.c
@@ -208,7 +208,8 @@ static nipc_uds_client_config_t service_client_config_to_transport(
     transport.preferred_profiles = config->preferred_profiles;
     transport.max_request_batch_items = config->max_request_batch_items;
     transport.max_response_payload_bytes = config->max_response_payload_bytes;
-    transport.max_response_batch_items = config->max_request_batch_items;
+    transport.max_response_batch_items =
+        nipc_service_common_typed_response_batch_items(config->max_request_batch_items);
     transport.auth_token = config->auth_token;
 
     return transport;
@@ -226,7 +227,8 @@ static nipc_uds_server_config_t service_server_config_to_transport(
     transport.preferred_profiles = config->preferred_profiles;
     transport.max_request_batch_items = config->max_request_batch_items;
     transport.max_response_payload_bytes = config->max_response_payload_bytes;
-    transport.max_response_batch_items = config->max_request_batch_items;
+    transport.max_response_batch_items =
+        nipc_service_common_typed_response_batch_items(config->max_request_batch_items);
     transport.auth_token = config->auth_token;
 
     return transport;
@@ -1341,7 +1343,7 @@ nipc_error_t nipc_server_init_cgroups_lookup(
     nipc_uds_server_config_t typed_cfg = service_server_config_to_transport(config);
     if (typed_cfg.max_request_payload_bytes == 0)
         typed_cfg.max_request_payload_bytes =
-            nipc_service_common_cgroups_response_payload_default();
+            nipc_service_common_lookup_request_payload_default();
     if (typed_cfg.max_response_payload_bytes == 0)
         typed_cfg.max_response_payload_bytes =
             nipc_service_common_cgroups_response_payload_default();
@@ -1371,7 +1373,7 @@ nipc_error_t nipc_server_init_apps_lookup(
     nipc_uds_server_config_t typed_cfg = service_server_config_to_transport(config);
     if (typed_cfg.max_request_payload_bytes == 0)
         typed_cfg.max_request_payload_bytes =
-            nipc_service_common_cgroups_response_payload_default();
+            nipc_service_common_lookup_request_payload_default();
     if (typed_cfg.max_response_payload_bytes == 0)
         typed_cfg.max_response_payload_bytes =
             nipc_service_common_cgroups_response_payload_default();
diff --git a/src/libnetdata/netipc/src/service/netipc_service_common.c b/src/libnetdata/netipc/src/service/netipc_service_common.c
@@ -57,6 +57,18 @@ uint32_t nipc_service_common_cgroups_response_payload_default(void)
     return NIPC_CLIENT_BUF_DEFAULT;
 }
 
+/* Lookup services batch dynamic keys, so their request buffer starts at the typed response size. */
+uint32_t nipc_service_common_lookup_request_payload_default(void)
+{
+    return NIPC_CLIENT_BUF_DEFAULT;
+}
+
+/* Typed services expose one batch-count knob; level 1 keeps request/response counts symmetric. */
+uint32_t nipc_service_common_typed_response_batch_items(uint32_t max_request_batch_items)
+{
+    return max_request_batch_items;
+}
+
 static void copy_cstr_field(char *dst, size_t dst_size, const char *src)
 {
     if (!src || dst_size == 0)
diff --git a/src/libnetdata/netipc/src/service/netipc_service_common.h b/src/libnetdata/netipc/src/service/netipc_service_common.h
@@ -29,6 +29,8 @@ bool nipc_service_common_header_payload_len_u32(uint32_t payload_len,
 bool nipc_service_common_mul_would_overflow(size_t count, size_t size);
 uint32_t nipc_service_common_cgroups_request_payload_default(void);
 uint32_t nipc_service_common_cgroups_response_payload_default(void);
+uint32_t nipc_service_common_lookup_request_payload_default(void);
+uint32_t nipc_service_common_typed_response_batch_items(uint32_t max_request_batch_items);
 
 void nipc_service_common_client_init(nipc_client_ctx_t *ctx,
                                      const char *run_dir,
diff --git a/src/libnetdata/netipc/src/service/netipc_service_win.c b/src/libnetdata/netipc/src/service/netipc_service_win.c
@@ -210,7 +210,8 @@ static nipc_np_client_config_t service_client_config_to_transport(
     transport.preferred_profiles = config->preferred_profiles;
     transport.max_request_batch_items = config->max_request_batch_items;
     transport.max_response_payload_bytes = config->max_response_payload_bytes;
-    transport.max_response_batch_items = config->max_request_batch_items;
+    transport.max_response_batch_items =
+        nipc_service_common_typed_response_batch_items(config->max_request_batch_items);
     transport.auth_token = config->auth_token;
 
     return transport;
@@ -228,7 +229,8 @@ static nipc_np_server_config_t service_server_config_to_transport(
     transport.preferred_profiles = config->preferred_profiles;
     transport.max_request_batch_items = config->max_request_batch_items;
     transport.max_response_payload_bytes = config->max_response_payload_bytes;
-    transport.max_response_batch_items = config->max_request_batch_items;
+    transport.max_response_batch_items =
+        nipc_service_common_typed_response_batch_items(config->max_request_batch_items);
     transport.auth_token = config->auth_token;
 
     return transport;
@@ -1339,7 +1341,7 @@ nipc_error_t nipc_server_init_cgroups_lookup(
     nipc_np_server_config_t typed_cfg = service_server_config_to_transport(config);
     if (typed_cfg.max_request_payload_bytes == 0)
         typed_cfg.max_request_payload_bytes =
-            nipc_service_common_cgroups_response_payload_default();
+            nipc_service_common_lookup_request_payload_default();
     if (typed_cfg.max_response_payload_bytes == 0)
         typed_cfg.max_response_payload_bytes =
             nipc_service_common_cgroups_response_payload_default();
@@ -1369,7 +1371,7 @@ nipc_error_t nipc_server_init_apps_lookup(
     nipc_np_server_config_t typed_cfg = service_server_config_to_transport(config);
     if (typed_cfg.max_request_payload_bytes == 0)
         typed_cfg.max_request_payload_bytes =
-            nipc_service_common_cgroups_response_payload_default();
+            nipc_service_common_lookup_request_payload_default();
     if (typed_cfg.max_response_payload_bytes == 0)
         typed_cfg.max_response_payload_bytes =
             nipc_service_common_cgroups_response_payload_default();

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ func clientConfigToTransport(config ClientConfig) posix.ClientConfig {`
`12`	`12`	`PreferredProfiles: config.PreferredProfiles,`
`13`	`13`	`MaxRequestBatchItems: config.MaxRequestBatchItems,`
`14`	`14`	`MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,`
`15`		`- MaxResponseBatchItems: config.MaxRequestBatchItems,`
	`15`	`+ MaxResponseBatchItems: typedResponseBatchItems(config.MaxRequestBatchItems),`
`16`	`16`	`AuthToken: config.AuthToken,`
`17`	`17`	`}`
`18`	`18`	`}`
`@@ -23,7 +23,7 @@ func serverConfigToTransport(config ServerConfig) posix.ServerConfig {`
`23`	`23`	`PreferredProfiles: config.PreferredProfiles,`
`24`	`24`	`MaxRequestBatchItems: config.MaxRequestBatchItems,`
`25`	`25`	`MaxResponsePayloadBytes: config.MaxResponsePayloadBytes,`
`26`		`- MaxResponseBatchItems: config.MaxRequestBatchItems,`
	`26`	`+ MaxResponseBatchItems: typedResponseBatchItems(config.MaxRequestBatchItems),`
`27`	`27`	`AuthToken: config.AuthToken,`
`28`	`28`	`}`
`29`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,11 @@ type Client struct {`
`9`	`9`	`inner *raw.Client`
`10`	`10`	`}`
`11`	`11`
	`12`	`+// typedResponseBatchItems keeps typed request/response batch counts symmetric.`
	`13`	`+func typedResponseBatchItems(maxRequestBatchItems uint32) uint32 {`
	`14`	`+ return maxRequestBatchItems`
	`15`	`+}`
	`16`	`+`
`12`	`17`	`func NewClient(runDir, serviceName string, config ClientConfig) *Client {`
`13`	`18`	`return &Client{inner: raw.NewAppsLookupClient(runDir, serviceName, clientConfigToTransport(config))}`
`14`	`19`	`}`