fix: prevent cyclic scene hierarchies

[Vansh0Sharma]

What does this PR do?

Fixes #975
Prevents 'parentId' updates from creating cyclic scene hierarchies.

• rejects reparenting a node beneath itself or one of its descendents.
• applies the guard in both 'apply-patch' and core scene mutation paths.
• adds global parent chain cycle detection to 'validate-scene' for imported or legacy scenes.
• adds regression coverage for the two site cycle reproduction.

How to test

1. run 'bun run test'
2. confirm the full suite completes successfully
3. verify the focused regression with :

   bun test \ 
packages/core/src/store/actions/reparent.test.ts \
packages/mcp/src/tools/apply-patch-identity.test.ys \
packages/mcp/src/tools/validate-scene.test.ts

4. confirm that reparenting Site_B under descendant Site_A is refused with invalid_parent an validate_scene reports an existing Site_A->Site_B->Site_B cycle.

## Screenshots / screen recording

not applicable - this is a core/mcp validation change with no visual UI change.

## Checklist

- [x] I've tested this locally with `bun dev`
- [x] My code follows the existing code style (run `bun check` to verify)
- [ ] I've updated relevant documentation (if applicable)
- [x] This PR targets the `main` branch

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Touches all reparent/create paths in the scene store and MCP patch guards; incorrect cycle logic could block valid reparents or miss edge cases, but changes are narrowly scoped to hierarchy checks with test coverage.
> 
> **Overview**
> Adds shared **parent-chain cycle detection** (`wouldCreateHierarchyCycle`, `findHierarchyCycles`) and wires it into scene mutations and MCP validation.
> 
> **Core store:** `parentId` changes on create and update (including batch `applyNodeChanges`) now **throw** before mutating if the new parent is the node itself, a descendant, or an already-cyclic chain—so reparents fail atomically with no partial graph update.
> 
> **MCP:** `apply_patch` dry-run rejects the same reparents as **`invalid_parent`**; `validate_scene` / bridge validation also flags **existing cycles** in imported or legacy scenes (per-node `parentId` errors with a cycle path), beyond per-node Zod checks.
> 
> Regression tests cover site A↔B cycles in the store, `apply_patch`, and `validate_scene`.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c5b51f5121fa2eae9b84f7ef3cd4ff7948dd9855. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

[pascal]

I hit an error while handling your request (Model unavailable on AI Gateway free tier: Free tier users do not have access to this model. Upgrade to paid credits at https://vercel.com/d?to=%2F%5Bteam%5D%2F%7E%2Fai%3Fmodal%3Dtop-up for unrestricted…).

Please try again, rephrase, or reach out if it keeps failing.

Error id: 235eccc9-a8c2-434e-9271-1e6a05796267

[ovurrsl]

Architectural Review: PR #980 (fix: prevent cyclic scene hierarchies)

Layer & Package Boundary Analysis

• packages/core (lib/hierarchy.ts, store/actions/node-actions.ts): Pure domain logic and graph algorithms with zero dependencies on Three.js, R3F, viewer, or editor packages. Complies fully with Invariant E-001.
• packages/mcp (bridge/scene-bridge.ts): Consumes findHierarchyCycles from @pascal-app/core cleanly to report diagnostics in validateScene().
• Composition & Registry: Respects the registry-driven model; hierarchy constraints remain orthogonal to node kind definitions.

---

Findings

Blockers: 0

None. The implementation cleanly adheres to all package boundaries and store invariants.

Suggestions: 1

• packages/core/src/store/actions/node-actions.ts:
  During multi-node batch operations (applyNodeChangesActionImpl and createNodesActionImpl), nextNodes is updated incrementally as each node is processed. The step-by-step invocation of wouldCreateHierarchyCycle(newNode.id, effectiveParentId, nextNodes) guarantees that both direct self-references and intra-batch multi-node loops are caught before commit.
  Suggestion: Consider adding a unit test specifically asserting that a 2-node circular swap within a single applyNodeChanges batch call is cleanly rejected with the exact error message.

Nits: 0

None. Code is concise, types are strict (NodeMap pick on parentId), and time complexity ($O(D)$ for incremental checks, $O(N)$ for global validation) is optimal.

---

Verification Summary

• Algorithmic Soundness: The dual approach — fast $O(D)$ ancestor walk for live mutations (wouldCreateHierarchyCycle) paired with linear $O(N)$ cycle detection for scene imports (findHierarchyCycles) — provides optimal performance without regression risks to high-frequency transform updates.
• Regression Coverage: reparent.test.ts and apply-patch-identity.test.ts test suites provide clean regression locks.

Verdict: Ready to merge (Fully complies with Pascal architectural guidelines).