From 6632c5c7b9740f8da4d05b7372bb0150f4e80f5c Mon Sep 17 00:00:00 2001 From: AriusII Date: Mon, 21 Sep 2026 21:24:24 +0200 Subject: [PATCH] Harden Client roadmap hierarchy validation --- docs/engineering/backlog.json | 4 +- docs/engineering/work-items/CLI-PLAN.md | 2 +- eng/Validate-EngineeringManifest.py | 129 +++++++++++++++--- .../test_validate_engineering_manifest.py | 66 +++++++++ 4 files changed, 178 insertions(+), 23 deletions(-) diff --git a/docs/engineering/backlog.json b/docs/engineering/backlog.json index d68115e..e28a5d8 100644 --- a/docs/engineering/backlog.json +++ b/docs/engineering/backlog.json @@ -54,7 +54,7 @@ "Navigable roadmap, detailed backlog and verified deployment receipt." ], "mandatory": "Governance root; close only when the defined roadmap scope is accepted or deliberately superseded.", - "evidence": "Proposal; initial GitHub writes were denied by the integration.", + "evidence": "MetadataRead + Proposal: the initial GitHub preparation attempt was denied (Resource not accessible by integration). A later operator-authorized import created and populated the private CheatEngineNet — Engineering Execution Project #1 and created Client issues #8–#40. This is a planning-metadata receipt only; it does not establish implementation, package, fixture, or live-host qualification.", "children": [ "CLI-E01", "CLI-E02", @@ -77,7 +77,7 @@ "path": "docs/engineering/work-items/CLI-PLAN.md", "long_term_impact": "One traceable owner, stable acceptance criteria, and explicit compatibility evidence reduce repeated integration fixes and make future API changes reviewable.", "benefits": "Connect outcome milestones, owned epics, implementable issues, source evidence, package gates and focused PRs.", - "body_template": "\n\n## CLI-PLAN — Establish the Client developer-workflow roadmap and execution baseline\n\n**Owner:** `CheatEngineNet/CheatEngine.Client` · **Kind:** roadmap · **Priority:** P1 · **Milestone:** Cross-phase navigation\n\n**Initial status:** Needs refinement. **Runtime validation:** Not executed by this bootstrap.\n\n### Context and evidence\n\nThis is the navigation root for the engineering bootstrap, not an implemented feature or a GitHub Project object.\n\n**Evidence classification:** Proposal; initial GitHub writes were denied by the integration.\n\n### Outcome and rationale\n\nConnect outcome milestones, owned epics, implementable issues, source evidence, package gates and focused PRs.\n\n**Expected benefit:** Connect outcome milestones, owned epics, implementable issues, source evidence, package gates and focused PRs.\n\n### Architectural responsibility\n\nSDK owns CE mappings, native safety, factual outcomes and low-level owners. Client owns application policy, typed workflows, composition, and developer experience.\n\n### Technical requirements and task checklist\n\n- [ ] Keep one owning repository per implementation task.\n- [ ] Keep hierarchy, blocking dependencies, package gates and release qualification distinct.\n- [ ] Never close this root automatically from one child implementation PR.\n\n### Acceptance criteria\n\n- [ ] Every work item links to its owning epic and verified sources.\n- [ ] Native GitHub relationships and deployment status are recorded accurately.\n- [ ] No speculative due dates, assignees, release versions or live-verification results are assigned.\n\n### Required validation\n\n- [ ] Manifest integrity, hierarchy and cross-repository dependency validation.\n\n### Scope exclusions\n\n- No automatic implementation branch farm, auto-merge, release or protection change.\n\n### Parent and children\n\nRepository roadmap root; not a GitHub Projects object.\n\n- {{issue:CLI-E01}}\n- {{issue:CLI-E02}}\n- {{issue:CLI-E03}}\n- {{issue:CLI-E04}}\n- {{issue:CLI-E05}}\n- {{issue:CLI-E06}}\n- {{issue:CLI-E07}}\n- {{issue:CLI-E08}}\n\n### Blocked by\n\nNone declared. This does not waive evidence, policy, or package requirements.\n\n### Blocks\n\nNone declared. This does not waive evidence, policy, or package requirements.\n\n### Package and readiness gate\n\nIdentify the first containing artifact before describing new source as consumable support. No version is invented by this plan.\n\n### Proposed branch and PR\n\nNo epic-sized implementation branch. Children have focused branch/PR proposals. The bootstrap creates only one documentation/tooling branch and draft PR per repository.\n\n### Risks and compatibility\n\nAn attractive board cannot substitute for precise contracts, acceptance evidence, and maintainable issue scopes.\n\n**Long-term impact:** One traceable owner, stable acceptance criteria, and explicit compatibility evidence reduce repeated integration fixes and make future API changes reviewable.\n\n### Definition of done\n\n- [ ] Navigable roadmap, detailed backlog and verified deployment receipt.\n\nGovernance root; close only when the defined roadmap scope is accepted or deliberately superseded.\n\n### Sources\n\n- [S01 — Existing build/test, focused branches and release rules; preserve rather than overwrite.](https://github.com/CheatEngineNet/CheatEngine.SDK/blob/aa3fcc3cdf629468e69d0c68817183d44a719894/CONTRIBUTING.md)\n- [C01 — Current in-process architecture, capability gates and managed deployment.](https://github.com/CheatEngineNet/CheatEngine.Client/blob/923a4ded85898f53ef4cd2ff5872d2fd9001071a/README.md)\n- [G01 — Issue API; filter pull_request entries from issue lists.](https://docs.github.com/en/rest/issues/issues)\n- [G02 — Repository-specific milestones.](https://docs.github.com/en/rest/issues/milestones)\n- [G03 — Native hierarchy uses issue IDs, not issue numbers.](https://docs.github.com/en/rest/issues/sub-issues)\n- [G04 — Native blocked-by relationship with the blocker issue_id.](https://docs.github.com/en/rest/issues/issue-dependencies)\n\n### Maintainer notes\n\nKeep execution updates, actual commands/results, decisions, and refinements here. The bootstrap does not overwrite an existing issue body on rerun.\n" + "body_template": "\n\n## CLI-PLAN — Establish the Client developer-workflow roadmap and execution baseline\n\n**Owner:** `CheatEngineNet/CheatEngine.Client` · **Kind:** roadmap · **Priority:** P1 · **Milestone:** Cross-phase navigation\n\n**Initial status:** Needs refinement. **Runtime validation:** Not executed by this bootstrap.\n\n### Context and evidence\n\nThis is the navigation root for the engineering bootstrap, not an implemented feature or a GitHub Project object.\n\n**Evidence classification:** `MetadataRead` + `Proposal`: the initial GitHub preparation attempt was denied (`Resource not accessible by integration`). A later operator-authorized import created and populated the private CheatEngineNet — Engineering Execution Project #1 and created Client issues #8–#40. This is a planning-metadata receipt only; it does not establish implementation, package, fixture, or live-host qualification.\n\n### Outcome and rationale\n\nConnect outcome milestones, owned epics, implementable issues, source evidence, package gates and focused PRs.\n\n**Expected benefit:** Connect outcome milestones, owned epics, implementable issues, source evidence, package gates and focused PRs.\n\n### Architectural responsibility\n\nSDK owns CE mappings, native safety, factual outcomes and low-level owners. Client owns application policy, typed workflows, composition, and developer experience.\n\n### Technical requirements and task checklist\n\n- [ ] Keep one owning repository per implementation task.\n- [ ] Keep hierarchy, blocking dependencies, package gates and release qualification distinct.\n- [ ] Never close this root automatically from one child implementation PR.\n\n### Acceptance criteria\n\n- [ ] Every work item links to its owning epic and verified sources.\n- [ ] Native GitHub relationships and deployment status are recorded accurately.\n- [ ] No speculative due dates, assignees, release versions or live-verification results are assigned.\n\n### Required validation\n\n- [ ] Manifest integrity, hierarchy and cross-repository dependency validation.\n\n### Scope exclusions\n\n- No automatic implementation branch farm, auto-merge, release or protection change.\n\n### Parent and children\n\nRepository roadmap root; not a GitHub Projects object.\n\n- {{issue:CLI-E01}}\n- {{issue:CLI-E02}}\n- {{issue:CLI-E03}}\n- {{issue:CLI-E04}}\n- {{issue:CLI-E05}}\n- {{issue:CLI-E06}}\n- {{issue:CLI-E07}}\n- {{issue:CLI-E08}}\n\n### Blocked by\n\nNone declared. This does not waive evidence, policy, or package requirements.\n\n### Blocks\n\nNone declared. This does not waive evidence, policy, or package requirements.\n\n### Package and readiness gate\n\nIdentify the first containing artifact before describing new source as consumable support. No version is invented by this plan.\n\n### Proposed branch and PR\n\nNo epic-sized implementation branch. Children have focused branch/PR proposals. The bootstrap creates only one documentation/tooling branch and draft PR per repository.\n\n### Risks and compatibility\n\nAn attractive board cannot substitute for precise contracts, acceptance evidence, and maintainable issue scopes.\n\n**Long-term impact:** One traceable owner, stable acceptance criteria, and explicit compatibility evidence reduce repeated integration fixes and make future API changes reviewable.\n\n### Definition of done\n\n- [ ] Navigable roadmap, detailed backlog and verified deployment receipt.\n\nGovernance root; close only when the defined roadmap scope is accepted or deliberately superseded.\n\n### Sources\n\n- [S01 — Existing build/test, focused branches and release rules; preserve rather than overwrite.](https://github.com/CheatEngineNet/CheatEngine.SDK/blob/aa3fcc3cdf629468e69d0c68817183d44a719894/CONTRIBUTING.md)\n- [C01 — Current in-process architecture, capability gates and managed deployment.](https://github.com/CheatEngineNet/CheatEngine.Client/blob/923a4ded85898f53ef4cd2ff5872d2fd9001071a/README.md)\n- [G01 — Issue API; filter pull_request entries from issue lists.](https://docs.github.com/en/rest/issues/issues)\n- [G02 — Repository-specific milestones.](https://docs.github.com/en/rest/issues/milestones)\n- [G03 — Native hierarchy uses issue IDs, not issue numbers.](https://docs.github.com/en/rest/issues/sub-issues)\n- [G04 — Native blocked-by relationship with the blocker issue_id.](https://docs.github.com/en/rest/issues/issue-dependencies)\n\n### Maintainer notes\n\nKeep execution updates, actual commands/results, decisions, and refinements here. The bootstrap does not overwrite an existing issue body on rerun.\n" }, { "id": "CLI-E01", diff --git a/docs/engineering/work-items/CLI-PLAN.md b/docs/engineering/work-items/CLI-PLAN.md index f32806d..c0f12be 100644 --- a/docs/engineering/work-items/CLI-PLAN.md +++ b/docs/engineering/work-items/CLI-PLAN.md @@ -10,7 +10,7 @@ This is the navigation root for the engineering bootstrap, not an implemented feature or a GitHub Project object. -**Evidence classification:** Proposal; initial GitHub writes were denied by the integration. +**Evidence classification:** `MetadataRead` + `Proposal`: the initial GitHub preparation attempt was denied (`Resource not accessible by integration`). A later operator-authorized import created and populated the private CheatEngineNet — Engineering Execution Project #1 and created Client issues [#8–#40](https://github.com/CheatEngineNet/CheatEngine.Client/issues/8). This is a planning-metadata receipt only; it does not establish implementation, package, fixture, or live-host qualification. ### Outcome and rationale diff --git a/eng/Validate-EngineeringManifest.py b/eng/Validate-EngineeringManifest.py index 16c1ebc..70d5ba4 100644 --- a/eng/Validate-EngineeringManifest.py +++ b/eng/Validate-EngineeringManifest.py @@ -106,6 +106,113 @@ def visit(node: str) -> None: visit(item_id) +def ensure_acyclic_parent_hierarchy(items: Mapping[str, Mapping[str, Any]]) -> None: + """Fail with the cycle path when the single-parent hierarchy is cyclic.""" + state: dict[str, int] = {} + stack: list[str] = [] + + def visit(item_id: str) -> None: + item_state = state.get(item_id, 0) + if item_state == 1: + start = stack.index(item_id) + raise ValueError("Cyclic parent hierarchy: " + " -> ".join([*stack[start:], item_id])) + if item_state == 2: + return + + state[item_id] = 1 + stack.append(item_id) + parent = items[item_id]["parent"] + if parent is not None: + visit(parent) + stack.pop() + state[item_id] = 2 + + for item_id in items: + visit(item_id) + + +def ensure_connected_parent_hierarchy(items: Mapping[str, Mapping[str, Any]], roadmap_id: str) -> None: + """Fail when any planning item cannot be reached from the roadmap root.""" + visited: set[str] = set() + stack = [roadmap_id] + while stack: + item_id = stack.pop() + if item_id in visited: + continue + visited.add(item_id) + stack.extend(items[item_id]["children"]) + disconnected = sorted(set(items) - visited) + require( + not disconnected, + f"Hierarchy is disconnected from roadmap root {roadmap_id}: {', '.join(disconnected)}.", + ) + + +def validate_parent_hierarchy(items: Mapping[str, Mapping[str, Any]]) -> None: + """Require one connected roadmap-to-epic-to-leaf parent tree.""" + for item_id, item in items.items(): + parent = item["parent"] + require( + parent is None or (isinstance(parent, str) and parent in items), + f"{item_id}: parent references an unknown item.", + ) + children = item["children"] + require(isinstance(children, list), f"{item_id}: children must be an array.") + require( + all(isinstance(child, str) and child in items for child in children), + f"{item_id}: children references an unknown item.", + ) + require(len(children) == len(set(children)), f"{item_id}: children must not contain duplicates.") + + ensure_acyclic_parent_hierarchy(items) + + for item_id, item in items.items(): + parent = item["parent"] + if parent is not None: + require(item_id in items[parent]["children"], f"{item_id}: parent does not list this child.") + for child in item["children"]: + require(items[child]["parent"] == item_id, f"{item_id}: child {child} has a different parent.") + + roadmap_ids = [item_id for item_id, item in items.items() if item["kind"] == "roadmap"] + require( + len(roadmap_ids) == 1, + f"Hierarchy must contain exactly one roadmap root; found {len(roadmap_ids)} roadmap items.", + ) + roadmap_id = roadmap_ids[0] + root_ids = [item_id for item_id, item in items.items() if item["parent"] is None] + require( + root_ids == [roadmap_id], + f"Hierarchy must have exactly one parentless root, roadmap {roadmap_id}; found {', '.join(root_ids) or 'none'}.", + ) + + roadmap = items[roadmap_id] + require(roadmap["milestone"] is None and roadmap["epic"] is None, f"{roadmap_id}: roadmap root cannot have a milestone or epic.") + for child in roadmap["children"]: + require(items[child]["kind"] == "epic", f"{roadmap_id}: roadmap child {child} must be an epic.") + + for item_id, item in items.items(): + if item_id == roadmap_id: + continue + if item["kind"] == "epic": + require(item["parent"] == roadmap_id, f"{item_id}: epic parent must be roadmap root {roadmap_id}.") + require(item["epic"] is None, f"{item_id}: an epic cannot belong to another epic.") + for child in item["children"]: + require( + items[child]["kind"] not in {"roadmap", "epic"}, + f"{item_id}: epic child {child} must be an implementation leaf.", + ) + else: + epic_id = item["epic"] + require( + isinstance(epic_id, str) and epic_id in items and items[epic_id]["kind"] == "epic", + f"{item_id}: leaf must name an existing epic.", + ) + require(item["parent"] == epic_id, f"{item_id}: leaf parent and epic must agree.") + require(not item["children"], f"{item_id}: implementation leaves cannot have children.") + + ensure_connected_parent_hierarchy(items, roadmap_id) + + def validate_manifest_data(data: Mapping[str, Any]) -> tuple[dict[str, dict[str, Any]], dict[str, int]]: """Validate backlog schema and local/cross-repository dependency invariants.""" require(data.get("schema_version") == 1, "backlog.json: schema_version must be 1.") @@ -215,30 +322,12 @@ def validate_manifest_data(data: Mapping[str, Any]) -> tuple[dict[str, dict[str, require((blocker, blocked) not in external_edges, f"Duplicate external dependency: {blocker} -> {blocked}.") external_edges.add((blocker, blocked)) + validate_parent_hierarchy(items) + local_dependencies: dict[str, list[str]] = {} for item_id, item in items.items(): - parent = item["parent"] - require(parent is None or parent in items, f"{item_id}: parent references an unknown item.") - if parent is not None: - require(item_id in items[parent]["children"], f"{item_id}: parent does not list this child.") - for child in item["children"]: - require(child in items, f"{item_id}: child references an unknown item: {child!r}.") - require(items[child]["parent"] == item_id, f"{item_id}: child {child} has a different parent.") milestone = item["milestone"] require(milestone is None or milestone in milestone_ids, f"{item_id}: unknown milestone {milestone!r}.") - if item["kind"] == "roadmap": - require( - parent is None and milestone is None and item["epic"] is None, - f"{item_id}: roadmap must be the hierarchy root.", - ) - elif item["kind"] == "epic": - require(item["epic"] is None, f"{item_id}: an epic cannot belong to another epic.") - else: - require( - item["epic"] in items and items[item["epic"]]["kind"] == "epic", - f"{item_id}: leaf must name an existing epic.", - ) - require(parent == item["epic"], f"{item_id}: leaf parent and epic must agree.") local_dependencies[item_id] = [] for blocker in item["blocked_by"]: diff --git a/eng/tests/test_validate_engineering_manifest.py b/eng/tests/test_validate_engineering_manifest.py index 0b1da2d..2bc5573 100644 --- a/eng/tests/test_validate_engineering_manifest.py +++ b/eng/tests/test_validate_engineering_manifest.py @@ -59,6 +59,72 @@ def test_cycle_is_rejected(self) -> None: with self.assertRaisesRegex(ValueError, "Cyclic local blocked_by dependency"): VALIDATOR.ensure_acyclic(dependencies) + def test_parent_cycle_is_rejected_independently_of_blocked_by(self) -> None: + manifest = copy.deepcopy(load_manifest()) + first_epic = next(item for item in manifest["items"] if item["id"] == "CLI-E01") + second_epic = next(item for item in manifest["items"] if item["id"] == "CLI-E02") + first_epic["parent"] = second_epic["id"] + second_epic["parent"] = first_epic["id"] + first_epic["children"].append(second_epic["id"]) + second_epic["children"].append(first_epic["id"]) + + with self.assertRaisesRegex(ValueError, "Cyclic parent hierarchy: CLI-E01 -> CLI-E02 -> CLI-E01"): + VALIDATOR.validate_manifest_data(manifest) + + def test_parent_hierarchy_requires_one_roadmap_root(self) -> None: + manifest = copy.deepcopy(load_manifest()) + epic = next(item for item in manifest["items"] if item["id"] == "CLI-E01") + epic["kind"] = "roadmap" + epic["labels"].append("ce:kind:roadmap") + + with self.assertRaisesRegex(ValueError, "exactly one roadmap root; found 2 roadmap items"): + VALIDATOR.validate_manifest_data(manifest) + + def test_parent_hierarchy_rejects_a_disconnected_forest(self) -> None: + hierarchy = { + "CLI-PLAN": {"children": []}, + "CLI-E01": {"children": []}, + } + + with self.assertRaisesRegex(ValueError, "Hierarchy is disconnected from roadmap root CLI-PLAN: CLI-E01"): + VALIDATOR.ensure_connected_parent_hierarchy(hierarchy, "CLI-PLAN") + + def test_roadmap_children_must_be_epics(self) -> None: + manifest = copy.deepcopy(load_manifest()) + roadmap = next(item for item in manifest["items"] if item["id"] == "CLI-PLAN") + epic = next(item for item in manifest["items"] if item["id"] == "CLI-E01") + leaf = next(item for item in manifest["items"] if item["id"] == "CLI-001") + roadmap["children"].append(leaf["id"]) + epic["children"].remove(leaf["id"]) + leaf["parent"] = roadmap["id"] + + with self.assertRaisesRegex(ValueError, "CLI-PLAN: roadmap child CLI-001 must be an epic"): + VALIDATOR.validate_manifest_data(manifest) + + def test_epics_must_be_direct_children_of_the_roadmap_root(self) -> None: + manifest = copy.deepcopy(load_manifest()) + roadmap = next(item for item in manifest["items"] if item["id"] == "CLI-PLAN") + first_epic = next(item for item in manifest["items"] if item["id"] == "CLI-E01") + second_epic = next(item for item in manifest["items"] if item["id"] == "CLI-E02") + roadmap["children"].remove(first_epic["id"]) + second_epic["children"].append(first_epic["id"]) + first_epic["parent"] = second_epic["id"] + + with self.assertRaisesRegex(ValueError, "CLI-E01: epic parent must be roadmap root CLI-PLAN"): + VALIDATOR.validate_manifest_data(manifest) + + def test_implementation_leaves_cannot_have_children(self) -> None: + manifest = copy.deepcopy(load_manifest()) + epic = next(item for item in manifest["items"] if item["id"] == "CLI-E01") + leaf = next(item for item in manifest["items"] if item["id"] == "CLI-001") + nested_leaf = next(item for item in manifest["items"] if item["id"] == "CLI-002") + epic["children"].remove(nested_leaf["id"]) + leaf["children"].append(nested_leaf["id"]) + nested_leaf["parent"] = leaf["id"] + + with self.assertRaisesRegex(ValueError, "CLI-001: implementation leaves cannot have children"): + VALIDATOR.validate_manifest_data(manifest) + def test_reverse_dependency_is_required(self) -> None: manifest = copy.deepcopy(load_manifest()) cli_001 = next(item for item in manifest["items"] if item["id"] == "CLI-001")