Reject non-directory action input parents

[tamird]

Superseded by #30926. The reproduced build failure requires preserving access to valid remote inputs through their implied directories; rejecting the input under a stale local file leaves that failure unchanged. The replacement also repairs physical output parents when preparing outputs.

Original proposal (superseded):

RemoteActionFileSystem looks up input metadata by full path, so a no-follow stat can return metadata for blocked/child even when blocked is a local file. Require a directory when canonicalizing the parent, using its cached node type and preserving the check through symlink resolution, so statIfFound reports absence. Deleting a child below a file continues to return false.

— Codex, on behalf of @tamird.

[tamird]

@tjgq this is the next step after #31311 (extracted per your request). Could you please take a look?

[tjgq]

Under what conditions can we end up in a state where the same path is a local file and the parent directory of a remote file? Do you have repro case in the form of a BUILD file and two consecutive Bazel commands to run?

(I'm worried that there's a broken invariant elsewhere in Bazel and we're just papering over it here.)

[tamird]

Following this through to the consumers exposed a real build failure, and this PR needs to be reworked.

With minimal downloads, three successive builds can produce this state:

1. Build a remote-only output blocked/child.
2. Build a separate target that writes a local file at blocked.
3. Reuse the cached remote child as an input to another action.

The third build succeeds for remote and local spawn consumers, but template expansion fails with Not a directory, and an executable symlink action rejects the child as “not a file.” These results reproduce with and without the disk cache. Applying this PR leaves both failures unchanged.

For template expansion, full-follow canonicalization encounters the local parent file before reaching the child's input metadata. That makes isRemote return false, so getInputStream skips the download that would repair the physical directory and attempts a local read. This PR makes no-follow stat report the child missing too.

The cached remote input is still valid: the remote consumer can use it, and local spawn prefetch repairs the directory. The violated contract is the action filesystem's ability to expose that declared input to in-process actions. It needs to preserve access through the input's implied parents and reconcile disk state when contents are materialized. The physical-parent precedence in #30926 also needs revisiting. A diagnostic change to that precedence and the remote executable check made all four consumer cases pass, but physical symlink semantics and the other directory operations still need to be worked through before that is a complete fix.

Reproducer and test setup

The consumer cases were tested in BuildWithoutTheBytesIntegrationTest using its real remote-worker harness, with --remote_download_minimal and disk cache both enabled and disabled. This is the Starlark setup used by that test; mode selects the consumer.

a/defs.bzl:

def _write_file_impl(ctx):
    out = ctx.actions.declare_file(ctx.attr.path)
    ctx.actions.run_shell(
        outputs = [out],
        command = "echo contents > " + out.path + " && chmod +x " + out.path,
        execution_requirements = {"local": "1"} if ctx.attr.local else {},
    )
    return [DefaultInfo(files = depset([out]))]

write_file = rule(
    implementation = _write_file_impl,
    attrs = {"path": attr.string(), "local": attr.bool()},
)

def _consume_impl(ctx):
    out = ctx.actions.declare_file("result")
    if ctx.attr.mode == "template":
        ctx.actions.expand_template(
            template = ctx.file.src,
            output = out,
            substitutions = {},
        )
    elif ctx.attr.mode == "symlink":
        ctx.actions.symlink(
            output = out,
            target_file = ctx.file.src,
            is_executable = True,
        )
    else:
        ctx.actions.run_shell(
            inputs = [ctx.file.src],
            outputs = [out],
            command = "cat " + ctx.file.src.path + " > " + out.path,
            execution_requirements = {"local": "1"} if ctx.attr.mode == "local" else {},
        )
    return [DefaultInfo(files = depset([out]))]

consume = rule(
    implementation = _consume_impl,
    attrs = {
        "src": attr.label(allow_single_file = True),
        "mode": attr.string(),
    },
)

a/BUILD:

load(":defs.bzl", "consume", "write_file")

write_file(name = "old", path = "blocked", local = True)
write_file(name = "child", path = "blocked/child")
consume(name = "consumer", src = ":child", mode = "template")

The harness calls buildTarget for //a:child, //a:old, and //a:consumer, waiting for downloads after each build. It checks that the child is absent locally after the first build and that blocked is a physical file after the second.

The equivalent CLI sequence, with REMOTE_EXECUTOR set to a working executor URI, is below. The template/symlink failures above were verified through the integration harness; the separate shell test exercised the successful remote-consumer case.

bazel build --remote_executor="$REMOTE_EXECUTOR" --remote_download_outputs=minimal //a:child
bazel build --remote_executor="$REMOTE_EXECUTOR" --remote_download_outputs=minimal //a:old
bazel build --remote_executor="$REMOTE_EXECUTOR" --remote_download_outputs=minimal //a:consumer

— Codex, on behalf of @tamird.

[tamird]

Closed in favor of #30926 (11d1eb4). The reproducer in the review thread shows that cached remote input metadata remains valid when a previous build leaves a local file at a parent path. This PR would hide that valid input from no-follow stat while leaving template expansion and executable-symlink actions broken.

#30926 now exposes the directories implied by checked inputs, preserves physical symlink traversal, allows on-demand downloads to repair stale physical parents, and repairs host paths during output preparation. It includes the reproducer as an integration test and no longer depends on this PR.

— Codex, on behalf of @tamird.