build(pem): make the direct-query kill switch a bool_flag with local_defines

Review feedback on #2401.

The BUILD comment advertised
`--//src/vizier/services/agent/pem:direct_query=disabled`, but no such
target existed — `bazel query` fails with "target 'direct_query' not
declared" — so the only working off-switch was `--define`. Declare the
bool_flag the comment promised and key the config_setting off it, so the
documented invocation works and the setting stays scoped to the targets
that read it instead of the global --define configuration key.

Also switches cc_library from `defines` to `local_defines` so the macro
stops propagating to every transitive dependent. Because that macro no
longer reaches dependents, direct_query_server_test selects on the same
config_setting itself — without that its `#ifdef PX_PEM_DIRECT_QUERY_DISABLED`
assertions would silently compile the enabled branch in a disabled build.

Verified with --config=x86_64_sysroot: default build compiles the feature
in (34 passed / 3 skipped), --//…:direct_query=false compiles the macro
into both the library and the test, and the resulting binary behaves
identically to the previous --define path (38 tests, 7 pass, 28 fail —
unchanged from the old mechanism; those failures are a separate issue).

NOTE: --define=PX_PEM_DIRECT_QUERY=disabled no longer has any effect.

Author: constanze

src/vizier/services/agent/pem/BUILD.bazel

@@ -14,22 +14,34 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
+load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
 load("@io_bazel_rules_docker//cc:image.bzl", "cc_image")
 load("@io_bazel_rules_docker//container:container.bzl", "container_image")
 load("//bazel:pl_build_system.bzl", "pl_cc_binary", "pl_cc_library", "pl_cc_test")
 
 package(default_visibility = ["//src/vizier:__subpackages__"])
 
-# Compile-time kill switch for the direct-query endpoint.
-# Operators who do not want the feature available even as a disabled-by-default
-# option build with `bazel build … --//src/vizier/services/agent/pem:direct_query=disabled`
-# (or `--define=PX_PEM_DIRECT_QUERY=disabled`). This propagates
-# `-DPX_PEM_DIRECT_QUERY_DISABLED` into cc_library; the entire feature body
-# in direct_query_server.cc and pem_manager.cc is `#ifndef`'d out and
-# stub methods return UNIMPLEMENTED. See DIRECT_QUERY_SECURITY.md.
+# Compile-time kill switch for the direct-query endpoint. Defaults to True, so
+# the feature is compiled in and gated at runtime by --direct_query_enabled
+# (itself false by default). Operators who do not want the feature present in
+# the binary at all build with
+# `bazel build … --//src/vizier/services/agent/pem:direct_query=false`, which
+# puts `-DPX_PEM_DIRECT_QUERY_DISABLED` on this package's compilations only:
+# the feature body in direct_query_server.cc and pem_manager.cc is `#ifndef`'d
+# out and stub methods return UNIMPLEMENTED. See DIRECT_QUERY_SECURITY.md.
+#
+# A build setting rather than `--define`: `--define` lives in the global
+# configuration key, so it applies to every target in the build and shows up in
+# every dependency's configuration; this flag is scoped to the targets that
+# read it.
+bool_flag(
+    name = "direct_query",
+    build_setting_default = True,
+)
+
 config_setting(
     name = "direct_query_disabled",
-    define_values = {"PX_PEM_DIRECT_QUERY": "disabled"},
+    flag_values = {":direct_query": "False"},
 )
 
 pl_cc_library(
@@ -42,7 +54,11 @@ pl_cc_library(
         ],
     ),
     hdrs = glob(["*.h"]),
-    defines = select({
+    # local_defines, not defines: the macro only guards this package's sources,
+    # so there is no reason to push it onto every transitive dependent's
+    # compilation. Targets that need to compile against the disabled build
+    # (direct_query_server_test below) select on the same config_setting.
+    local_defines = select({
         ":direct_query_disabled": ["PX_PEM_DIRECT_QUERY_DISABLED"],
         "//conditions:default": [],
     }),
@@ -88,6 +104,14 @@ pl_cc_test(
 pl_cc_test(
     name = "direct_query_server_test",
     srcs = ["direct_query_server_test.cc"],
+    # The kill-switch assertions are `#ifdef PX_PEM_DIRECT_QUERY_DISABLED`, and
+    # cc_library's local_defines deliberately do not reach here — so select on
+    # the flag again, otherwise a `--//…:direct_query=false` run would silently
+    # compile the enabled branch and skip the coverage entirely.
+    defines = select({
+        ":direct_query_disabled": ["PX_PEM_DIRECT_QUERY_DISABLED"],
+        "//conditions:default": [],
+    }),
     deps = [
         ":cc_library",
         "//src/api/proto/vizierpb:vizier_pl_cc_proto",

src/vizier/services/agent/pem/DIRECT_QUERY_SECURITY.md

@@ -226,12 +226,15 @@ When `--direct_query_enabled=false`:
 
 ```bash
 bazel build //src/vizier/services/agent/pem:pem_image \
-    --define=PX_PEM_DIRECT_QUERY=disabled
+    --//src/vizier/services/agent/pem:direct_query=false
 ```
 
-When `PX_PEM_DIRECT_QUERY_DISABLED` is defined at compile time
-(propagated by the `:direct_query_disabled` `config_setting` in
-`src/vizier/services/agent/pem/BUILD.bazel`):
+The `:direct_query` `bool_flag` defaults to `True`, so ordinary builds
+compile the feature in and rely on the runtime flag above. Setting it to
+`false` matches the `:direct_query_disabled` `config_setting` in
+`src/vizier/services/agent/pem/BUILD.bazel`, which puts
+`PX_PEM_DIRECT_QUERY_DISABLED` on this package's compilations (and on the
+endpoint's test target). When that macro is defined:
 
 - The **entire feature-bearing body** of `direct_query_server.cc` is
   excluded via `#ifndef`: no openssl HMAC, no rapidjson, no JWT

src/vizier/services/agent/pem/direct_query_server_test.cc

@@ -816,7 +816,7 @@ TEST_F(DirectQueryServerExecTest, FailSoft_BrokerFailureToleratedByDirectQuery)
 // stand it up here; the visible contract is: the runtime flag's early return
 // short-circuits before line 1 of feature code runs.
 //
-// Compile-time toggle (PX_PEM_DIRECT_QUERY=disabled) is asserted in code
+// Compile-time toggle (--//src/vizier/services/agent/pem:direct_query=false) is asserted in code
 // below: when compiled with the macro, AuthenticateRequest and
 // DirectQueryServer::ExecuteScript both return
 // UNAUTHENTICATED/UNIMPLEMENTED unconditionally, independent of token or
@@ -852,7 +852,8 @@ TEST_F(DirectQueryServerTest, CompiledOut_NoToken_Unauthenticated) {
 TEST_F(DirectQueryServerTest, ToggleContract_DocumentBothLevels) {
   SUCCEED() << "Default build: runtime --direct_query_enabled gates port :50305 "
                "binding (pem_manager.cc:MaybeStartDirectQueryServer early-returns). "
-               "Compile-time PX_PEM_DIRECT_QUERY=disabled additionally drops all "
+               "Compile-time --//src/vizier/services/agent/pem:direct_query=false "
+               "additionally drops all "
                "feature bytes from the binary (no JWT verifier, no Carnot driver, "
                "no openssl/rapidjson includes). See DIRECT_QUERY_SECURITY.md.";
 }