Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 56 additions & 19 deletions scripts/ci/reborn_pr_test_plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,25 +156,10 @@ def _sandbox_docker_prefixes() -> tuple[str, ...]:
"scripts/telegram_smoke/",
)
CHANGED_COVERAGE_MANIFEST = "tests/integration/changed-coverage-exemptions.toml"
SANDBOX_DOCKER_EXACT_PATHS = {
# Path classes with no owning crate: no relocation can ever change them, so
# they stay plain literals.
SANDBOX_DOCKER_EXACT_PATH_LITERALS = (
"Dockerfile.sandbox-worker",
"crates/app/ironclaw_cli/src/runtime/mod.rs",
"crates/app/ironclaw_composition/src/sandbox.rs",
"crates/app/ironclaw_composition/src/builtin_capability_policy.rs",
"crates/app/ironclaw_composition/src/deployment.rs",
"crates/app/ironclaw_composition/src/factory/production_backend_assembly.rs",
"crates/app/ironclaw_composition/src/factory/runtime_lane_assembly.rs",
"crates/app/ironclaw_composition/src/input.rs",
"crates/app/ironclaw_config/src/profile.rs",
"crates/kernel/ironclaw_host_runtime/src/first_party_tools/mod.rs",
"crates/kernel/ironclaw_host_runtime/src/invocation_services.rs",
"crates/kernel/ironclaw_host_runtime/src/process_port.rs",
"crates/kernel/ironclaw_host_runtime/src/services.rs",
"crates/kernel/ironclaw_host_runtime/src/services/builder.rs",
"crates/kernel/ironclaw_runtime_policy/src/planner.rs",
"crates/kernel/ironclaw_runtime_policy/src/resolver.rs",
"crates/lanes/ironclaw_sandbox/tests/support/docker_gate.rs",
"crates/lanes/ironclaw_sandbox/tests/user_sandbox_docker_live.rs",
"tests/integration/reborn_sandbox_shell_turn.rs",
"tests/e2e_trace_runtime_policy_serde.rs",
"tests/fixtures/llm_traces/runtime_policy/hosted_dev_no_shell.json",
Expand All @@ -184,7 +169,59 @@ def _sandbox_docker_prefixes() -> tuple[str, ...]:
"tests/integration/support/harness/mod.rs",
"tests/integration/support/harness/options.rs",
"tests/integration/support/harness/profiles/sandbox_shell.rs",
)
# Crate-relative suffixes for entries that live inside a workspace crate's own
# directory. `_sandbox_docker_exact_paths()` resolves each crate's root by
# name through the shared inventory (scripts/ci/lib/crate_tree.py), the same
# mechanism `_sandbox_docker_prefixes()` above already uses for
# `ironclaw_sandbox` — a hardcoded `crates/<family>/<crate>` prefix stops
# matching the moment any of these crates is relocated, and the planner would
# then silently stop routing that crate's listed files to the Docker lane.
SANDBOX_DOCKER_EXACT_PATH_CRATE_SUFFIXES: dict[str, tuple[str, ...]] = {
"ironclaw_cli": ("src/runtime/mod.rs",),
"ironclaw_composition": (
"src/sandbox.rs",
"src/builtin_capability_policy.rs",
"src/deployment.rs",
"src/factory/production_backend_assembly.rs",
"src/factory/runtime_lane_assembly.rs",
"src/input.rs",
),
"ironclaw_config": ("src/profile.rs",),
"ironclaw_host_runtime": (
"src/first_party_tools/mod.rs",
"src/invocation_services.rs",
"src/process_port.rs",
"src/services.rs",
"src/services/builder.rs",
),
"ironclaw_runtime_policy": (
"src/planner.rs",
"src/resolver.rs",
),
"ironclaw_sandbox": (
"tests/support/docker_gate.rs",
"tests/user_sandbox_docker_live.rs",
),
}


def _sandbox_docker_exact_paths() -> set[str]:
"""Exact-match sandbox Docker paths, crate-owned entries resolved by name."""
paths = set(SANDBOX_DOCKER_EXACT_PATH_LITERALS)
for crate, suffixes in SANDBOX_DOCKER_EXACT_PATH_CRATE_SUFFIXES.items():
try:
directory = crate_directory(crate, ROOT)
except CrateTreeError as error:
raise RuntimeError(
f"reborn_pr_test_plan: cannot resolve the {crate} crate, so "
"the exact source paths used to route the Docker lane are "
f"unknown: {error}"
) from error
paths.update(f"{directory}/{suffix}" for suffix in suffixes)
return paths


# Asset trees that live outside every crate root but are compiled *into* a
# workspace crate through a relative `include_bytes!` / `include_str!` that
# escapes its own crate (the §11.2.7 reach-ins inventoried by
Expand Down Expand Up @@ -724,7 +761,7 @@ def build_plan(
root_inventory = _root_test_partitions()
integration_inventory = _integration_test_lanes()
sandbox_docker_prefixes = _sandbox_docker_prefixes()
sandbox_docker_exact_paths = set(SANDBOX_DOCKER_EXACT_PATHS)
sandbox_docker_exact_paths = _sandbox_docker_exact_paths()
if sandbox_docker_prefixes:
sandbox_crate_directory = sandbox_docker_prefixes[0].removesuffix(
"/src/sandbox_process"
Expand Down
96 changes: 96 additions & 0 deletions scripts/ci/test_reborn_pr_test_plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,9 @@ def test_frontend_prefix_resolves_through_crate_inventory_when_nested(self) -> N
try:
with (
mock.patch.object(planner, "_sandbox_docker_prefixes", return_value=()),
mock.patch.object(
planner, "_sandbox_docker_exact_paths", return_value=set()
),
mock.patch.object(
planner,
"crate_directory",
Expand All @@ -406,6 +409,9 @@ def test_frontend_prefix_resolution_failure_fails_closed(self) -> None:
try:
with (
mock.patch.object(planner, "_sandbox_docker_prefixes", return_value=()),
mock.patch.object(
planner, "_sandbox_docker_exact_paths", return_value=set()
),
mock.patch.object(
planner,
"crate_directory",
Expand Down Expand Up @@ -590,6 +596,96 @@ def test_sandbox_docker_prefix_follows_crate_inventory_when_nested(self) -> None
self.assertTrue(plan["run_sandbox_docker"])
resolver.assert_any_call("ironclaw_sandbox", planner.ROOT)

def test_sandbox_docker_exact_paths_resolve_through_crate_inventory_when_nested(
self,
) -> None:
"""A family-moved crate among the six sandbox-Docker exact-path owners
still contributes its entries at the new location, and the old
location's entries stop appearing — the same relocation-safety
`_sandbox_docker_prefixes` already provides for `ironclaw_sandbox`.
"""
from crate_tree import crate_directory as real_crate_directory

moved_directory = "crates/kernel-next/ironclaw_runtime_policy"

def fake_crate_directory(name: str, root: Path = planner.ROOT) -> str:
if name == "ironclaw_runtime_policy":
return moved_directory
return real_crate_directory(name, root)

with mock.patch.object(
planner, "crate_directory", side_effect=fake_crate_directory
):
exact_paths = planner._sandbox_docker_exact_paths()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Test relocated exact paths through build_plan

This regression test calls _sandbox_docker_exact_paths() directly, so it remains green if build_plan() stops consuming the resolved set or fails to set run_sandbox_docker for a relocated path—the exact silent loss of Docker coverage this change is intended to prevent. Drive build_plan() with the relocated planner.rs/resolver.rs paths and assert the emitted Docker-lane flag instead of only inspecting the helper result.

AGENTS.md reference: AGENTS.md:L78-L83

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed, and fixed in 77923ed. I reproduced the gap with a mutation test first: I removed the path in sandbox_docker_exact_paths arm from build_plan()'s consumption check (keeping the prefix arm intact) and reran the suite. test_sandbox_docker_exact_paths_resolve_through_crate_inventory_when_nested stayed green because it only asserts on _sandbox_docker_exact_paths()'s return value, never touching build_plan(). That's the exact silent-coverage-loss scenario this PR exists to prevent.

Added test_sandbox_docker_exact_paths_route_relocated_crate_through_build_plan, following the same pattern as test_sandbox_docker_prefix_follows_crate_inventory_when_nested: it mocks crate_directory to relocate ironclaw_runtime_policy and drives build_plan() itself instead of calling the resolver directly. With the mutation in place this new test fails (run_sandbox_docker is False); reverting the mutation makes it pass again.

One deliberate detail: the test also mutates the synthetic package's manifest_path to the relocated directory. Without that, build_plan()'s package-directory lookup can't find the moved crate, hits the "a crate path maps to no workspace package" arm, and falls back to _full_plan() — which sets run_sandbox_docker=True unconditionally regardless of the exact-paths logic. That fallback would make the assertion pass for the wrong reason and mask a real regression. The test also asserts plan["mode"] == "selected" (not "full") specifically to rule that out.

Reran both suites clean: test_reborn_pr_test_plan.py (75/75) and test_ws12_workflow_contracts.py (52/52).


self.assertIn(f"{moved_directory}/src/planner.rs", exact_paths)
self.assertIn(f"{moved_directory}/src/resolver.rs", exact_paths)
self.assertNotIn(
"crates/kernel/ironclaw_runtime_policy/src/planner.rs", exact_paths
)
self.assertNotIn(
"crates/kernel/ironclaw_runtime_policy/src/resolver.rs", exact_paths
)
# An unrelated owner's entries are untouched by the relocation.
self.assertIn("crates/app/ironclaw_cli/src/runtime/mod.rs", exact_paths)

def test_sandbox_docker_exact_paths_route_relocated_crate_through_build_plan(
self,
) -> None:
"""The sibling test above pins that a relocated crate's entries
appear in `_sandbox_docker_exact_paths()`'s returned set. That alone
would stay green even if `build_plan()` stopped consuming the
resolved set or stopped setting `run_sandbox_docker` for a matched
path — the exact silent loss of Docker coverage this change exists
to prevent. Drive `build_plan()` itself, the same way
`test_sandbox_docker_prefix_follows_crate_inventory_when_nested`
already does for `_sandbox_docker_prefixes`, and assert the emitted
Docker-lane flag."""
from crate_tree import crate_directory as real_crate_directory

moved_directory = "crates/kernel-next/ironclaw_runtime_policy"
moved_metadata = metadata()
next(
package
for package in moved_metadata["packages"]
if package["id"] == "alpha"
)["manifest_path"] = str(ROOT / moved_directory / "Cargo.toml")

def fake_crate_directory(name: str, root: Path = planner.ROOT) -> str:
if name == "ironclaw_runtime_policy":
return moved_directory
return real_crate_directory(name, root)

with mock.patch.object(
planner, "crate_directory", side_effect=fake_crate_directory
):
plan = planner.build_plan(
event="pull_request",
changed_paths=[f"{moved_directory}/src/planner.rs"],
metadata=moved_metadata,
canonical_packages=self.canonical,
)

# `mode` must be "selected", not "full" — a package-resolution miss
# falls back to the exhaustive plan (see `build_plan`'s "a crate path
# maps to no workspace package" arm), which also sets
# `run_sandbox_docker`, but for the wrong reason and without
# exercising the exact-path routing this test targets.
self.assertEqual(plan["mode"], "selected")
self.assertTrue(plan["run_sandbox_docker"])

def test_sandbox_docker_exact_paths_resolution_failure_fails_closed(self) -> None:
"""An unresolvable crate must raise, never silently drop that crate's
entries from the exact-path set — the same fail-closed contract as
`_sandbox_docker_prefixes` and `_webui_frontend_prefix`."""
with mock.patch.object(
planner, "crate_directory", side_effect=planner.CrateTreeError("boom")
):
with self.assertRaisesRegex(
RuntimeError, "cannot resolve the ironclaw_cli crate"
):
planner._sandbox_docker_exact_paths()

def test_sandbox_docker_paths_preserve_regular_test_inventory_selection(
self,
) -> None:
Expand Down