refactor: route Bazel docs inputs through the CLI

bzl/needs_rules.bzl

@@ -32,9 +32,10 @@ def _sphinx_docs_impl(ctx):
     if not bundle.own_source_files.to_list():
         fail("Sphinx requires a bundle with direct documentation sources")
 
-    # Expand file labels at analysis time, then encode the argument list as
-    # JSON so spaces, quotes and '=' in Sphinx options survive the environment
-    # transport unchanged. The launcher adds these after its default options.
+    # File labels provide execroot-relative paths for this action's sandbox.
+    # Pass them through the environment variables already consumed by the CLI
+    # and extensions; reserve the JSON option list for non-path Sphinx overrides.
+    # Encode that list as JSON so spaces, quotes and '=' survive transport.
     # ``config`` is transported separately because the launcher derives
     # Sphinx's ``-c`` directory from its path; it is not just another data file.
     env = {
@@ -43,10 +44,16 @@ def _sphinx_docs_impl(ctx):
         "OUTPUT_DIRECTORY": output.path,
         "SPHINX_CONFIG_FILE": ctx.file.config.path,
         "DATA": "[]",
-        "SPHINX_EXTRA_OPTS": json.encode([
-            ctx.expand_location(option, targets = ctx.attr.tools)
-            for option in ctx.attr.extra_opts
-        ]),
+        "SCORE_SOURCELINKS": (
+            ctx.file.score_sourcelinks_json.path if ctx.file.score_sourcelinks_json else ""
+        ),
+        "MOUNTS_MANIFEST": (
+            ctx.file.mounts_manifest.path if ctx.file.mounts_manifest else ""
+        ),
+        "SCORE_METAMODEL_YAML": (
+            ctx.file.score_metamodel_yaml.path if ctx.file.score_metamodel_yaml else ""
+        ),
+        "SPHINX_EXTRA_OPTS": json.encode(ctx.attr.extra_opts),
     }
 
     # Data and mounted sources must be present at their execution-root paths.
@@ -56,7 +63,15 @@ def _sphinx_docs_impl(ctx):
         executable = ctx.executable.sphinx,
         env = env,
         inputs = depset(
-            [ctx.file.config] + ctx.files.data + ctx.files.tools,
+            [ctx.file.config] + ctx.files.data + ctx.files.tools + [
+                file
+                for file in [
+                    ctx.file.score_sourcelinks_json,
+                    ctx.file.mounts_manifest,
+                    ctx.file.score_metamodel_yaml,
+                ]
+                if file
+            ],
             transitive = [bundle.own_source_files],
         ),
         outputs = [output],
@@ -73,6 +88,11 @@ sphinx_docs = rule(
         "bundle": attr.label(providers = [DocsBundleInfo], mandatory = True),
         "data": attr.label_list(allow_files = True),
         "tools": attr.label_list(allow_files = True),
+        # Typed labels let the action pass their execroot paths through the
+        # environment contract above and still declare sandbox inputs.
+        "score_sourcelinks_json": attr.label(allow_single_file = True),
+        "mounts_manifest": attr.label(allow_single_file = True),
+        "score_metamodel_yaml": attr.label(allow_single_file = True),
         "extra_opts": attr.string_list(),
         # The launcher runs on the build host and carries extension runfiles.
         "sphinx": attr.label(cfg = "exec", executable = True, mandatory = True),

docs.bzl

@@ -81,10 +81,7 @@ def _needs_sphinx_extra_opts(
         master_doc,
         external_needs_source,
         score_bundle_needs_export,
-        score_sourcelinks_json,
-        score_source_code_linker_plain_links,
-        mounts_manifest,
-        score_metamodel_yaml):
+        score_source_code_linker_plain_links):
     """Return per-target Sphinx configuration defines for a Needs build."""
     # The launcher supplies diagnostics shared by every builder. Keep only
     # target-specific defines here so the action does not receive duplicate
@@ -95,10 +92,7 @@ def _needs_sphinx_extra_opts(
         ("master_doc", master_doc),
         ("external_needs_source", external_needs_source),
         ("score_bundle_needs_export", score_bundle_needs_export),
-        ("score_sourcelinks_json", score_sourcelinks_json),
         ("score_source_code_linker_plain_links", score_source_code_linker_plain_links),
-        ("mounts_manifest", mounts_manifest),
-        ("score_metamodel_yaml", score_metamodel_yaml),
         ]
         for option in _sphinx_define(name, value)
     ]
@@ -135,6 +129,14 @@ def _needs_sphinx_docs(
         sphinx_build_data = [],
         visibility = None):
     """Declare a bundle Needs export with the repository-wide Sphinx policy."""
+    # These three are consumed as their own typed rule attributes (below), not
+    # as ordinary tools; still list them here so the caller does not have to
+    # repeat them when building its own ``tools`` list.
+    tools = tools + [
+        label
+        for label in [score_sourcelinks_json, mounts_manifest, score_metamodel_yaml]
+        if label
+    ]
     sphinx_build = _declare_sphinx_build_binary(
         name,
         sphinx_build_data + [tool for tool in tools if tool not in sphinx_build_data],
@@ -152,11 +154,14 @@ def _needs_sphinx_docs(
             master_doc,
             external_needs_source,
             score_bundle_needs_export,
-            score_sourcelinks_json,
             score_source_code_linker_plain_links,
-            mounts_manifest,
-            score_metamodel_yaml,
         ),
+        # Keep these as labels rather than path strings in ``extra_opts``. The
+        # private rule declares them as action inputs and provides execroot
+        # paths directly through the environment.
+        score_sourcelinks_json = score_sourcelinks_json,
+        mounts_manifest = mounts_manifest,
+        score_metamodel_yaml = score_metamodel_yaml,
         sphinx = sphinx_build,
         tools = tools,
         visibility = visibility,
@@ -354,6 +359,9 @@ def _declare_bundle_local_needs(
     sphinx_build_deps = _sphinx_runtime_deps(deps)
 
     needs_local = _bundle_internal_target(name, "needs_local")
+    # The generated source-links target stays typed as a label here; the
+    # private Needs rule owns translating it to an action environment path
+    # and declaring it as an input.
     _needs_sphinx_docs(
         name = needs_local,
         bundle = ":" + name,
@@ -363,9 +371,8 @@ def _declare_bundle_local_needs(
         master_doc = entry_doc,
         external_needs_source = "[]",
         score_bundle_needs_export = "1",
-        score_sourcelinks_json = "$(location " + str(sourcelinks_json) + ")" if sourcelinks_json else None,
+        score_sourcelinks_json = sourcelinks_json,
         score_source_code_linker_plain_links = "1",
-        tools = [sourcelinks_json] if sourcelinks_json else [],
         visibility = visibility,
     )
 
@@ -547,20 +554,18 @@ def docs(
     # list-valued attributes such as ``data`` and ``tools``.
     metamodel_label = [metamodel] if metamodel else []
 
-    mounts_manifest_label = []
+    mounts_manifest = None
     if bundles:
         mounts_bundle = create_bundle(
             name = "_docs_mounts",
             bundles = bundles,
             visibility = ["//visibility:private"],
         )
-
-        mounts_manifest_label = [
-            create_mounts_manifest(
-                name = "_mounts_manifest",
-                bundle = mounts_bundle,
-            ),
-        ]
+        mounts_manifest = create_mounts_manifest(
+            name = "_mounts_manifest",
+            bundle = mounts_bundle,
+        )
+    mounts_manifest_label = [mounts_manifest] if mounts_manifest else []
 
     deps = _sphinx_deps(deps)
     deps = deps + [
@@ -626,8 +631,8 @@ def docs(
         "EXTERNAL_NEEDS_FILES": str(external_needs),
         # `bazel run` starts from a runfiles tree, so this logical path is
         # resolved by score_mounts through ``RUNFILES_DIR``.
-        "MOUNTS_MANIFEST": "$(rlocationpath :_mounts_manifest)" if bundles else "",
-        "SCORE_SOURCELINKS": "$(location :sourcelinks_json)",
+        "MOUNTS_MANIFEST": "$(rlocationpath :_mounts_manifest)" if mounts_manifest else "",
+        "SCORE_SOURCELINKS": "$(rlocationpath :sourcelinks_json)",
     }
     if config_is_generated:
         # The generated file is named conf.py. Run targets pass its containing
@@ -639,7 +644,7 @@ def docs(
         docs_env["SCORE_METAMODEL_YAML"] = "$(rlocationpath " + str(metamodel) + ")"
     if known_good_label:
         known_good_str = str(known_good_label[0])
-        docs_env["KNOWN_GOOD_JSON"] = "$(location " + known_good_str + ")"
+        docs_env["KNOWN_GOOD_JSON"] = "$(rlocationpath " + known_good_str + ")"
         docs_data += known_good_label
 
     # Generated documentation artifacts may live below ``docs/``.  A
@@ -697,13 +702,11 @@ def docs(
         sphinx_build_deps = deps,
         sphinx_build_data = data + external_needs + metamodel_label + [":docs_bundle"],
         external_needs_source = str(data + external_needs),
-        score_sourcelinks_json = "$(location :sourcelinks_json)",
+        score_sourcelinks_json = ":sourcelinks_json",
         score_source_code_linker_plain_links = "1",
-        # The build action runs in a sandbox, so it needs the action-input path
-        # rather than the runfiles-relative spelling.
-        mounts_manifest = "$(location :_mounts_manifest)" if bundles else None,
-        score_metamodel_yaml = "$(location " + str(metamodel) + ")" if metamodel else None,
-        tools = external_needs + metamodel_label + [":sourcelinks_json", ":docs_bundle"] + mounts_manifest_label,
+        mounts_manifest = mounts_manifest,
+        score_metamodel_yaml = metamodel,
+        tools = external_needs + [":docs_bundle"],
         visibility = ["//visibility:public"],
     )
 

src/docs_cli/cli.py

@@ -138,11 +138,30 @@ def add_watch_dir(path: Path) -> None:
     return watch_dirs
 
 
+def _resolve_runfiles_relative_path(config: DocsCliConfig, value: Path) -> Path:
+    """Resolve a Bazel-provided path that may be runfiles-relative.
+
+    Build actions and direct calls already receive an absolute or
+    execroot/cwd-relative path. ``bazel run`` instead passes an
+    rlocationpath, which must be joined with the launcher's own runfiles
+    directory before use.
+    """
+    if not config.is_bazel_build and not value.is_absolute():
+        runfiles_dir = env.optional_path("RUNFILES_DIR")
+        ws_root = config.ws_root or Path()
+        value = runfiles_dir / value if runfiles_dir is not None else ws_root / value
+    return value.absolute()
+
+
 def sphinx_arguments(
     config: DocsCliConfig,
 ) -> list[str]:
     """Build Sphinx arguments from the resolved launcher configuration."""
     output_dir = config.output_dir
+    mounts_manifest = env.optional_path("MOUNTS_MANIFEST")
+    if mounts_manifest:
+        mounts_manifest = _resolve_runfiles_relative_path(config, mounts_manifest)
+
     base_arguments = [
         str(config.source_dir),
         str(output_dir),
@@ -157,7 +176,7 @@ def sphinx_arguments(
         f"--define=testcase_source_dirs={env.get('TEST_SOURCES', '[]')}",
         # Path to the Bazel-emitted mounts manifest (empty when no mounts are
         # configured); consumed by the score_mounts extension.
-        f"--define=mounts_manifest={env.optional_path('MOUNTS_MANIFEST') or ''}",
+        f"--define=mounts_manifest={mounts_manifest or ''}",
     ]
 
     if config.is_bazel_build:
@@ -179,34 +198,19 @@ def sphinx_arguments(
         base_arguments.extend(["--warning-file", str(output_dir / "warnings.txt")])
 
     if config_file := env.optional_path("SPHINX_CONFIG_FILE"):
-        # The action receives ctx.file.config.path, which is interpreted from
-        # the action's execution-root working directory. Resolve it locally
-        # instead of using runfiles lookup; interactive targets receive a
-        # runfiles-relative path and need that lookup before Sphinx gets the
-        # containing directory.
-        if config.is_bazel_build:
-            config_file = config_file.absolute()
-        elif not config_file.is_absolute():
-            config_file = get_runfiles_dir() / config_file
+        config_file = _resolve_runfiles_relative_path(config, config_file)
         base_arguments.extend(["-c", str(config_file.parent)])
 
     if metamodel_yaml := env.optional_path("SCORE_METAMODEL_YAML"):
-        # Under ``bazel run``, this environment variable is runfiles-relative
-        # and must be resolved through RUNFILES_DIR. A sandboxed Needs action
-        # instead expands the metamodel label to an execution-root path in
-        # SPHINX_EXTRA_OPTS; applying runfiles lookup there would escape the
-        # action's declared inputs.
-        if not config.is_bazel_build and not metamodel_yaml.is_absolute():
-            runfiles_dir = env.optional_path("RUNFILES_DIR")
-            ws_root = config.ws_root or Path()
-            metamodel_yaml = (
-                runfiles_dir / metamodel_yaml
-                if runfiles_dir is not None
-                else ws_root / metamodel_yaml
-            )
-        metamodel_yaml = metamodel_yaml.absolute()
+        metamodel_yaml = _resolve_runfiles_relative_path(config, metamodel_yaml)
         base_arguments.append(f"--define=score_metamodel_yaml={metamodel_yaml}")
 
+    if sourcelinks_json := env.optional_path("SCORE_SOURCELINKS"):
+        # The sandboxed Needs action and ``bazel run`` both set this env var;
+        # only the extension reads ``app.config.score_sourcelinks_json``.
+        sourcelinks_json = _resolve_runfiles_relative_path(config, sourcelinks_json)
+        base_arguments.append(f"--define=score_sourcelinks_json={sourcelinks_json}")
+
     if github_repository := env.get("GITHUB_REPOSITORY", ""):
         # GITHUB_REPOSITORY is expected as "owner/repo"; partition("/") splits
         # once into (owner, separator, repo), so we can ignore the separator.
@@ -221,6 +225,7 @@ def sphinx_arguments(
         base_arguments.append(f"-A=doc_path={relative_doc_path}")
 
     if known_good_json := env.optional_path("KNOWN_GOOD_JSON"):
+        known_good_json = _resolve_runfiles_relative_path(config, known_good_json)
         base_arguments.append(f"--define=KNOWN_GOOD_JSON={known_good_json}")
 
     return base_arguments
@@ -231,17 +236,12 @@ def watch_arguments(config: DocsCliConfig) -> list[str]:
     mounts_manifest = env.optional_path("MOUNTS_MANIFEST")
     watch_arguments: list[str] = []
     if mounts_manifest:
-        # ``MOUNTS_MANIFEST`` is runfiles-relative under ``bazel run`` and
-        # an ordinary path for direct invocations, matching score_mounts.
-        manifest_path = (
-            get_runfiles_dir() / mounts_manifest
-            if config.is_bazel_run
-            else mounts_manifest
-        )
+        manifest_path = _resolve_runfiles_relative_path(config, mounts_manifest)
+        runfiles_dir = get_runfiles_dir() if config.is_bazel_run else None
         for watch_dir in mounted_watch_dirs(
             manifest_path,
             config.ws_root,
-            get_runfiles_dir() if config.is_bazel_run else None,
+            runfiles_dir,
         ):
             watch_arguments.extend(["--watch", watch_dir])
     return watch_arguments
@@ -277,6 +277,12 @@ def main(argv: list[str] | None = None) -> int:
         debugpy.wait_for_client()
 
     config = DocsCliConfig.from_environment(env)
+    # cli.py is only ever invoked via `bazel run` (a _declare_docs_binary
+    # target) or as the sandboxed Needs action's executable; see
+    # src/docs_cli/README.md. ExecutionEnvironment.DIRECT exists so
+    # DocsCliConfig/sphinx_arguments stay unit-testable without a real
+    # runfiles tree (see main_test.py) and should never occur here.
+    assert not config.is_direct, "cli.py must run via bazel run or a Bazel action"
     ws_root = config.ws_root or Path()
     package_dir = config.package_dir
     output_dir = config.output_dir