diff --git a/fixos/diagnostics/venv_cleanup.py b/fixos/diagnostics/venv_cleanup.py index 4c43326..0b1cb9d 100644 --- a/fixos/diagnostics/venv_cleanup.py +++ b/fixos/diagnostics/venv_cleanup.py @@ -119,7 +119,13 @@ def observe(field, callback, pid=process_pid): def _within(path: Path, parent: Path) -> bool: - return path == parent or parent in path.parents + # Every process path is compared with many projects and mounts. Walking + # all its ancestors repeatedly makes inventory needlessly expensive. + if os.path.normcase(path.anchor) != os.path.normcase(parent.anchor): + return False + return tuple(map(os.path.normcase, path.parts[: len(parent.parts)])) == tuple( + map(os.path.normcase, parent.parts) + ) def _measure_project_source(project: Path, exclude: tuple[str, ...]) -> dict: diff --git a/project/ticket-116/README.md b/project/ticket-116/README.md new file mode 100644 index 0000000..71e1f96 --- /dev/null +++ b/project/ticket-116/README.md @@ -0,0 +1,38 @@ +# Ticket 116: Avoid repeated ancestor walks in project containment checks + +- **ID**: ticket-116 +- **Owner**: agent:codex +- **Status**: IN_PROGRESS +- **Workflow state**: PUBLICATION +- **Created**: 2026-10-31 + +## Goal and scope + +Project inventory repeatedly compares every visible process path and mount +against project/artifact paths. The old containment helper constructs and +compares every ancestor each time. Live MCP inventory exceeded 90 seconds; +a diagnostic equivalent-prefix comparison completed the project phase in +9.3 seconds and state inventory in 20.2 seconds. Preserve lexical and native +case behavior while eliminating the repeated ancestor walk. + +SESSION_EXECUTION_AUTHORIZATION: the user requested execution of the remaining +FixOS/host repairs on 2026-10-31. This bounded fix includes independent protected +publication and deployment to both existing host installations. + +## Acceptance criteria + +- [x] AC-01: Equality, descendants, sibling boundaries, relative anchors, + lexical parent segments and native-case semantics match existing behavior. +- [x] AC-02: Unrelated deep paths do not enumerate their ancestors; affected + tests and managed governance pass with unchanged cleanup protections. +- [ ] AC-03: Required full checks, independent exact-head publication and + installed-module/MCP inventory verification succeed. + +## Validation + +The new ancestor-walk regression failed before the change. After the change, +56 affected tests pass, including all 14 containment tests. The affected +Testless coverage scan also passes all 56 tests. Ruff passes for both changed +implementation paths. Required full checks and independent publication bind +the final commit; live deployment will verify both installed module hashes +and the actual MCP project-inventory duration. diff --git a/project/ticket-116/intent.json b/project/ticket-116/intent.json new file mode 100644 index 0000000..f8a669d --- /dev/null +++ b/project/ticket-116/intent.json @@ -0,0 +1,76 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-116", + "summary": "Avoid repeated ancestor walks in project containment checks", + "workstream": "application", + "classification": { + "kind": "BUG", + "priority": "P1", + "origin": "requested" + }, + "allowedPaths": [ + "project/ticket-116/**", + "fixos/diagnostics/venv_cleanup.py", + "tests/unit/test_path_scope.py" + ], + "forbiddenPaths": [ + "project/ticket-*/user-*.md" + ], + "stacks": [ + "python" + ], + "dependsOn": [], + "conflictsWith": [], + "integrationTicket": null, + "delivery": { + "acceptedBaseSha": "1badd07decd568bc1012c823354c03ee62a0981b", + "targetBranch": "main", + "outcome": "Independently publish and deploy equivalent lexical containment checks that allow project inventory to complete on the live host without repeated ancestor traversal.", + "nonGoals": [ + "No cleanup eligibility, process protection, retention or storage-floor changes", + "No filesystem resolution or removal of unknown work", + "No MCP schema or new runtime dependency" + ], + "complexity": "XS", + "estimatedMinutes": 10, + "budgets": { + "maxImplementationFiles": 2, + "maxAffectedComponents": 1, + "maxPublicInterfaceChanges": 0, + "maxRuntimeDependencies": 0 + }, + "architecture": { + "status": "accepted", + "decision": "Compare native-case normalized anchors and path component prefixes, preserving lexical containment while avoiding Path.parents enumeration per process/project pair.", + "components": [ + { + "name": "project-path-scope", + "paths": [ + "fixos/diagnostics/venv_cleanup.py", + "tests/unit/test_path_scope.py" + ] + } + ], + "responsibilityChanges": false, + "interfaceChanges": [], + "dataChanges": [], + "ui": { + "impact": "none", + "states": [], + "evidence": [] + }, + "rollback": "Revert through independent protected publication and reinstall the previous reviewed artifact." + }, + "runtimeDependencies": [], + "validation": [ + { + "criterion": "AC-01", + "commands": [ + "python -m pytest tests/unit/test_path_scope.py tests/unit/test_venv_cleanup.py -q", + "./project/governance-check.sh" + ], + "evidence": "project/ticket-116/README.md" + } + ] + } +} diff --git a/tests/unit/test_path_scope.py b/tests/unit/test_path_scope.py new file mode 100644 index 0000000..ec7256b --- /dev/null +++ b/tests/unit/test_path_scope.py @@ -0,0 +1,48 @@ +"""Lexical containment must stay conservative without walking every ancestor.""" + +import os +from pathlib import Path + +import pytest + +from fixos.diagnostics.venv_cleanup import _within + + +@pytest.mark.parametrize( + ("path", "parent", "expected"), + [ + ("/", "/", True), + ("/repo", "/repo", True), + ("/repo/.venv/bin/python", "/repo", True), + ("/repo-old/file", "/repo", False), + ("/repo", "/repo/child", False), + ("/elsewhere/file", "/repo", False), + ("repo/child", "repo", True), + ("repo/child", ".", True), + ("/repo/child", ".", False), + ("repo/child", "/repo", False), + ("/repo/../elsewhere", "/repo", True), + ], +) +def test_existing_lexical_scope_boundaries(path, parent, expected): + assert _within(Path(path), Path(parent)) is expected + + +def test_native_case_rules_are_preserved(): + assert _within(Path("REPO/child"), Path("repo")) is (os.name == "nt") + + +def test_distinct_absolute_anchors_remain_distinct(): + assert Path("//host/share/file").anchor != Path("/host/share").anchor + assert not _within(Path("//host/share/file"), Path("/host/share")) + + +def test_unrelated_deep_path_does_not_enumerate_ancestors(): + class NoAncestorWalk(type(Path())): + @property + def parents(self): + raise AssertionError("Containment must not enumerate all ancestors") + + path = NoAncestorWalk("/unrelated/" + "/".join(["nested"] * 2048)) + + assert not _within(path, Path("/home/tom/github/project"))