Skip to content
Merged
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
96 changes: 41 additions & 55 deletions .github/workflows/pr-review-panel.lock.yml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion .github/workflows/triage-panel.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 10 additions & 9 deletions docs/src/content/docs/contributing/development-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -364,15 +364,16 @@ WIP push.
### Workflow dependency updates

When updating actions in generated `.github/workflows/*.lock.yml` files,
keep their `gh-aw-manifest` headers, human-readable action lists, and
`.github/aw/actions-lock.json` entries aligned with the runtime `uses:` pins.
Dependabot does not update those metadata records. Preserve the compiler
version and source hashes for dependency-only edits; recompile with
`gh aw compile` when changing workflow source.

Run `uv run --frozen --extra dev pytest tests/unit/test_triage_panel_lock.py`
to check setup and app-token action pin consistency across the manifest-bearing
workflows.
align `gh-aw-manifest` headers, action lists, and `.github/aw/actions-lock.json`
entries with runtime `uses:` pins. Dependabot does not update this metadata.
Preserve compiler versions and source hashes for dependency-only edits.
For source changes, including comments, run `gh aw compile` with the repository's
pinned compiler version from `.github/workflows/copilot-setup-steps.yml`.
Commit source and regenerated lock together; do not edit compiler metadata by hand.

Run `uv run --frozen --extra dev pytest tests/unit/test_triage_panel_lock.py tests/unit/test_shared_apm_workflow_contract.py`
to check triage/review source freshness across line endings, compiler-header
consistency with the repository pin, and action-pin consistency.

### Code scanning on pull requests and merge queues

Expand Down
4 changes: 3 additions & 1 deletion docs/src/content/docs/enterprise/drift-detection.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ lockfile-exists -> ref-consistency -> deployment-ledger-owners

After the baseline passes, it replays the install in a scratch directory
and diffs against the working tree to surface `unintegrated`, `modified`,
`orphaned`, and `unrecorded` files. `unrecorded` means replay produced the
`orphaned`, and `unrecorded` files. Root-local primitives use the same
source discovery scope as a normal install; the scratch directory is
only the deployment destination. `unrecorded` means replay produced the
same normalized bytes as the project but no exact or directory
`deployed_files` claim covers the path; shared merge-hook targets are exempt
from `unrecorded`. For their drift comparison, only APM-owned hook entries are
Expand Down
12 changes: 11 additions & 1 deletion src/apm_cli/integration/base_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -820,9 +820,19 @@ def _is_root_local_package(package_info, project_root: Path) -> bool:

Used to scope discovery to ``.apm/`` and ``.github/`` instead of
walking the entire project tree -- see issue #1507.

During drift replay *project_root* is the scratch directory;
fall back to ``root_local_project_root`` when set (same pattern
as ``HookIntegrator._is_root_local_package``).
"""
try:
return Path(package_info.install_path).resolve() == Path(project_root).resolve()
resolved_install = Path(package_info.install_path).resolve()
if resolved_install == Path(project_root).resolve():
return True
root_local = getattr(package_info, "root_local_project_root", None)
if root_local is not None:
return resolved_install == Path(root_local).resolve()
return False
except (OSError, RuntimeError):
return False

Expand Down
64 changes: 43 additions & 21 deletions tests/unit/integration/test_base_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -705,9 +705,12 @@ def test_scopes_to_apm_subdir_when_install_path_is_home(self, mock_resolver_cls,

mock_discover.assert_called_once_with(Path.home() / ".apm")

@pytest.mark.parametrize("has_recorded_root", [False, True])
@patch("apm_cli.integration.base_integrator.discover_primitives")
@patch("apm_cli.integration.base_integrator.UnifiedLinkResolver")
def test_uses_install_path_when_not_home(self, mock_resolver_cls, mock_discover, tmp_path):
def test_uses_install_path_when_not_home(
self, mock_resolver_cls, mock_discover, tmp_path, has_recorded_root
):
"""Real installed dependencies (install_path under apm_modules/, NOT
equal to project_root) must scan install_path directly."""
mock_discover.return_value = []
Expand All @@ -718,10 +721,15 @@ def test_uses_install_path_when_not_home(self, mock_resolver_cls, mock_discover,
install_path = tmp_path / "apm_modules" / "owner" / "repo"
install_path.mkdir(parents=True)
pkg_info.install_path = install_path
pkg_info.root_local_project_root = tmp_path if has_recorded_root else None
pkg_info.deployment_package_root = None

bi.init_link_resolver(pkg_info, tmp_path)

mock_discover.assert_called_once_with(install_path)
assert bi.link_resolver is mock_resolver_cls.return_value
assert bi.link_resolver.package_root == install_path
assert bi.link_resolver.deployment_package_root == install_path

@patch("apm_cli.integration.base_integrator.discover_primitives")
@patch("apm_cli.integration.base_integrator.UnifiedLinkResolver")
Expand Down Expand Up @@ -769,10 +777,26 @@ class TestInitLinkResolverLocalScoping:
not get walked end-to-end. See issue #1507 (13-minute hang).
"""

@pytest.fixture(params=["install", "replay"])
def local_context(
self,
tmp_path: Path,
tmp_path_factory: pytest.TempPathFactory,
request: pytest.FixtureRequest,
) -> tuple[MagicMock, Path]:
"""Keep the source root fixed when replay deploys into scratch."""
pkg_info = MagicMock()
pkg_info.install_path = tmp_path
pkg_info.deployment_package_root = None
replay = request.param == "replay"
pkg_info.root_local_project_root = tmp_path if replay else None
destination = tmp_path_factory.mktemp("replay") if replay else tmp_path
return pkg_info, destination

@patch("apm_cli.integration.base_integrator.discover_primitives")
@patch("apm_cli.integration.base_integrator.UnifiedLinkResolver")
def test_narrows_to_apm_and_github_when_install_path_is_project_root(
self, mock_resolver_cls, mock_discover, tmp_path
self, mock_resolver_cls, mock_discover, tmp_path, local_context
):
mock_discover.return_value = []
(tmp_path / ".apm").mkdir()
Expand All @@ -783,13 +807,12 @@ def test_narrows_to_apm_and_github_when_install_path_is_project_root(
(tmp_path / "noise" / "deep" / "irrelevant.txt").write_text("x")

bi = BaseIntegrator()
pkg_info = MagicMock()
pkg_info.install_path = tmp_path

bi.init_link_resolver(pkg_info, tmp_path)
bi.init_link_resolver(*local_context)

called_roots = [call.args[0] for call in mock_discover.call_args_list]
assert mock_resolver_cls.return_value.package_root == tmp_path
assert bi.link_resolver is mock_resolver_cls.return_value
assert bi.link_resolver.package_root == tmp_path
assert bi.link_resolver.deployment_package_root == tmp_path
assert tmp_path / ".apm" in called_roots
assert tmp_path / ".github" in called_roots
# Critically: project_root itself was NOT passed to discover_primitives.
Expand All @@ -800,37 +823,35 @@ def test_narrows_to_apm_and_github_when_install_path_is_project_root(

@patch("apm_cli.integration.base_integrator.discover_primitives")
@patch("apm_cli.integration.base_integrator.UnifiedLinkResolver")
def test_skips_missing_directories(self, mock_resolver_cls, mock_discover, tmp_path):
def test_skips_missing_directories(
self, mock_resolver_cls, mock_discover, tmp_path, local_context
):
"""If only ``.apm/`` exists, only ``.apm/`` is scanned -- no waste
from probing a non-existent ``.github/``."""
mock_discover.return_value = []
(tmp_path / ".apm").mkdir()
# No .github/

bi = BaseIntegrator()
pkg_info = MagicMock()
pkg_info.install_path = tmp_path

bi.init_link_resolver(pkg_info, tmp_path)
bi.init_link_resolver(*local_context)

called_roots = [call.args[0] for call in mock_discover.call_args_list]
assert bi.link_resolver is mock_resolver_cls.return_value
assert called_roots == [tmp_path / ".apm"]

@patch("apm_cli.integration.base_integrator.discover_primitives")
@patch("apm_cli.integration.base_integrator.UnifiedLinkResolver")
def test_no_apm_or_github_means_no_walk(self, mock_resolver_cls, mock_discover, tmp_path):
def test_no_apm_or_github_means_no_walk(self, mock_resolver_cls, mock_discover, local_context):
"""Project root with no .apm/ or .github/ must not walk anything."""
mock_discover.return_value = []

bi = BaseIntegrator()
pkg_info = MagicMock()
pkg_info.install_path = tmp_path

bi.init_link_resolver(pkg_info, tmp_path)
bi.init_link_resolver(*local_context)

mock_discover.assert_not_called()
assert bi.link_resolver is mock_resolver_cls.return_value

def test_real_walk_does_not_traverse_noise_subtree(self, tmp_path):
def test_real_walk_does_not_traverse_noise_subtree(self, tmp_path, local_context):
"""End-to-end: with a real (non-mocked) discover_primitives call,
confirm files under a noise subtree do NOT get walked. Acts as a
regression trap for the original 13-minute hang on large repos.
Expand Down Expand Up @@ -862,12 +883,13 @@ def spy_walk(top, *args, **kwargs):
yield dirpath, dirnames, filenames

bi = BaseIntegrator()
pkg_info = MagicMock()
pkg_info.install_path = tmp_path

with patch("apm_cli.primitives.discovery.os.walk", side_effect=spy_walk):
bi.init_link_resolver(pkg_info, tmp_path)
bi.init_link_resolver(*local_context)

assert bi.link_resolver is not None
assert bi.link_resolver.package_root == tmp_path
assert bi.link_resolver.deployment_package_root == tmp_path
# The noise subtree must never appear in any walked directory.
for d in visited_dirs:
assert "noise" not in Path(d).parts, f"discovery walked noise subtree: {d}"
Expand Down
23 changes: 17 additions & 6 deletions tests/unit/test_deps_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,23 +96,34 @@ def test_nested_multiple_levels_deep(self, tmp_path):
assert _is_nested_under_package(deep, modules) is True


@pytest.mark.windows_compat
@pytest.mark.parametrize("alias", [".safe", "safe.", "foo..bar", "my-skill.v2"])
def test_scan_includes_flattened_alias_without_nested_or_symlink_packages(
tmp_path: Path, alias: str
) -> None:
"""Prune must see an alias root, but not its contents or external links."""
def test_scan_includes_flattened_alias_without_nested_packages(tmp_path: Path, alias: str) -> None:
"""Prune must see an alias root, but not its nested packages."""
modules = tmp_path / "apm_modules"
package = modules / alias
package.mkdir(parents=True)
_make_apm_yml(package)
nested = package / "nested"
nested.mkdir()
_make_apm_yml(nested)
# Windows strips trailing dots when creating directories; scan the on-disk name.
assert _scan_installed_packages(modules) == [package.resolve().name]


@pytest.mark.windows_compat
def test_scan_excludes_symlink_packages(tmp_path: Path) -> None:
"""Symlink prerequisites must not skip the independent alias regression."""
modules = tmp_path / "apm_modules"
modules.mkdir()
outside = tmp_path / "outside"
outside.mkdir()
_make_apm_yml(outside)
(modules / "linked").symlink_to(outside, target_is_directory=True)
assert _scan_installed_packages(modules) == [alias]
try:
(modules / "linked").symlink_to(outside, target_is_directory=True)
except (NotImplementedError, OSError):
pytest.skip("platform does not support directory symlinks")
assert _scan_installed_packages(modules) == []


# ==================================================================
Expand Down
6 changes: 5 additions & 1 deletion tests/unit/test_shared_apm_workflow_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -766,9 +766,13 @@ def test_repository_pins_exact_gh_aw_compiler_and_generated_locks() -> None:
sources = sorted(workflows.glob("*.md"))
for source in sources:
lock = source.with_suffix(".lock.yml")
first_line = lock.read_text(encoding="utf-8").splitlines()[0]
lock_text = lock.read_text(encoding="utf-8")
first_line = lock_text.splitlines()[0]
metadata = json.loads(first_line.removeprefix("# gh-aw-metadata: "))
assert metadata["compiler_version"] == GH_AW_VERSION, source.name
generated = re.search(r"generated by gh-aw \((v[^)]+)\)", lock_text)
assert generated is not None, source.name
assert generated.group(1) == GH_AW_VERSION, source.name


def test_agentic_workflows_agent_is_deployed_verbatim() -> None:
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/test_triage_panel_lock.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Regression checks for generated workflow action pins and Triage Panel metadata."""

import hashlib
import json
import re
from pathlib import Path
Expand All @@ -19,6 +20,53 @@ def _load_lock_header(lock_text: str, prefix: str) -> dict:
return json.loads(matching_lines[0].removeprefix(prefix))


@pytest.mark.windows_compat
@pytest.mark.parametrize("workflow", ["triage-panel", "pr-review-panel"])
@pytest.mark.parametrize("newline", ["\n", "\r\n", "\r"], ids=["lf", "crlf", "cr"])
def test_advisory_lock_metadata_matches_source(workflow: str, newline: str, tmp_path: Path) -> None:
"""Catch stale source hashes before an advisory workflow reaches activation."""
for relative in (f"{workflow}.md", "shared/apm.md", f"{workflow}.lock.yml"):
target = tmp_path / relative
target.parent.mkdir(parents=True, exist_ok=True)
text = (LOCK_PATH.parent / relative).read_text(encoding="utf-8")
target.write_bytes(text.replace("\n", newline).encode("utf-8"))

# read_text uses universal-newline translation, including CRLF and bare CR.
source_path = tmp_path / f"{workflow}.md"
source = source_path.read_text(encoding="utf-8")
_, frontmatter, body = source.split("---", 2)
config = yaml.safe_load(frontmatter)
assert [item["uses"] for item in config["imports"]] == ["shared/apm.md"]
assert not config.get("inlined-imports")
assert not re.search(r"\$\{\{[^}]*(?:env\.|vars\.)", body)

imported = (source_path.parent / "shared/apm.md").read_text(encoding="utf-8")
_, imported_frontmatter, imported_body = imported.split("---", 2)
assert "imports" not in yaml.safe_load(imported_frontmatter)

# gh-aw v4 hashes normalized source text, not parsed YAML (comments count).
canonical = {
"frontmatter-text": frontmatter.strip(),
"imported-frontmatters": imported_frontmatter.strip(),
"imports": ["shared/apm.md"],
}
encoded = json.dumps(
canonical, sort_keys=True, separators=(",", ":"), ensure_ascii=False
).encode("utf-8")
combined_body = "\n---\n".join((body.strip(), imported_body.strip()))
lock_text = source_path.with_suffix(".lock.yml").read_text(encoding="utf-8")
metadata = _load_lock_header(lock_text, "# gh-aw-metadata: ")
assert metadata["schema_version"] == "v4"
recompile = f"Run gh aw compile {workflow} with its recorded compiler version."
assert metadata["frontmatter_hash"] == hashlib.sha256(encoded).hexdigest(), recompile
assert metadata["body_hash"] == hashlib.sha256(combined_body.encode("utf-8")).hexdigest(), (
recompile
)
generated_version = re.search(r"generated by gh-aw \((v[^)]+)\)", lock_text)
assert generated_version is not None
assert metadata["compiler_version"] == generated_version.group(1)


@pytest.mark.parametrize(
"workflow",
[
Expand Down
Loading