diff --git a/.cursor/commands/audit.md b/.cursor/commands/audit.md index c44b4393..c516b4b7 100644 --- a/.cursor/commands/audit.md +++ b/.cursor/commands/audit.md @@ -62,9 +62,9 @@ Syncback Core │ Parent, Source (scripts), Value (StringValue), Contents │ (LocalizationTable), Ref/UniqueId, unscriptable props. ├── src/syncback/ref_properties.rs Ref property resolution. collect_referents() finds all -│ Ref properties. link_referents() writes Rojo_Ref_* or -│ Rojo_Target_* attributes. Path-based preferred, falls -│ back to ID-based for ambiguous paths. +│ Ref properties. link_referents() writes Rojo_Ref_* +│ (path-based) and Rojo_Id attributes to instances. +│ Rojo_Target_* is legacy (read-only during forward sync). ├── src/syncback/hash/mod.rs Content hashing. hash_tree() hashes entire tree bottom- │ └── src/syncback/hash/variant.rs up with blake3. hash_instance() hashes class + filtered │ properties + children hashes. Ref properties hashed as @@ -82,9 +82,9 @@ Syncback Core Snapshot Middleware (syncback direction: instance → filesystem) ├── src/snapshot_middleware/mod.rs Dispatcher. Middleware enum routes to handler. -│ get_best_middleware() selects middleware for an instance. │ Each middleware has syncback() producing SyncbackReturn -│ { fs_snapshot, children }. +│ { fs_snapshot, children }. get_best_middleware() lives +│ in src/syncback/mod.rs (selects middleware for an instance). ├── src/snapshot_middleware/dir.rs Directory handler. syncback_dir() processes children, │ creates init.meta.json5, handles .gitkeep for empty dirs. │ syncback_dir_no_meta() handles child dedup and recursion. @@ -148,9 +148,10 @@ Change Batching (Plugin) │ Encodes a single property value using RbxDom.EncodedValue. │ encode(). Maps Roblox values to wire format. ├── plugin/src/ChangeBatcher/encodeInstance.lua -│ Full instance encoding for additions. Duplicate detection -│ (skips instances with duplicate-named siblings). -│ Recursive children encoding. Attributes/Tags support. +│ Full instance encoding for additions. All children are +│ encoded including those with duplicate names -- the server +│ handles dedup. Skips Ref properties (target has no server +│ ID during addition). Attributes/Tags support. └── plugin/src/ChangeBatcher/propertyFilter.lua Filters which properties are synced. Controls which data types and property names are included/excluded. @@ -176,12 +177,28 @@ Server Write Processing (receives plugin writes) │ model files. Sends PatchSet to tree_mutation_sender. │ CRITICAL: file writes and PatchSet must stay consistent. ├── src/change_processor.rs handle_tree_event() receives PatchSet from api.rs. -│ Processes removals (tree update), updates (Source writes, -│ renames with slugify/dedup/meta lifecycle, ClassName -│ transitions), additions (tree insert). Applies patch via -│ apply_patch_set(). Broadcasts via message_queue. +│ Processes removals (tree update), dedup cleanup (with +│ removed_set to exclude co-removed siblings), updates +│ (Source writes, renames with slugify/dedup/meta lifecycle, +│ ClassName transitions, ref path updates via RefPathIndex), +│ additions (tree insert). Applies patch via apply_patch_set(). +│ Broadcasts via message_queue. +├── src/git.rs Git integration. compute_git_metadata() identifies changed +│ files relative to HEAD. compute_blob_sha1() hashes content +│ using git blob format. git_add() stages files after writes. +│ Used by api.rs for stageIds and by serve_session for +│ GitMetadata in server info response. +├── src/rojo_ref.rs Ref path system. ref_target_path_from_tree() builds +│ filesystem-name paths for Rojo_Ref_* attributes. RefPathIndex +│ tracks which files contain which ref paths for efficient +│ updates on rename. Constants: REF_PATH_ATTRIBUTE_PREFIX, +│ REF_POINTER_ATTRIBUTE_PREFIX, REF_ID_ATTRIBUTE_NAME. +├── src/variant_eq.rs Property value comparison. variant_eq() compares Variant +│ values with fuzzy float matching (approx_eq! with epsilon). +│ Used by matching algorithms for property diff scoring. └── src/serve_session.rs Server-side session. Owns RojoTree, ChangeProcessor, - MessageQueue. Coordinates serve lifecycle. + MessageQueue, RefPathIndex, git_repo_root. Coordinates + serve lifecycle. SERVER → PLUGIN (Filesystem changes pushed to Studio) ===================================================== @@ -211,7 +228,12 @@ Snapshot Generation (Server, forward-sync direction: filesystem → instance) │ DirectoryMetadata types used as overlays. ├── src/snapshot_middleware/project.rs snapshot_project() reads .project.json5. Resolves nodes, │ infers class names for services, merges $properties. -└── (other middleware: csv.rs, txt.rs, json.rs, toml.rs, yaml.rs, rbxm.rs, rbxmx.rs) +├── (other middleware: csv.rs, txt.rs, json.rs, toml.rs, yaml.rs, rbxm.rs, rbxmx.rs) +│ +├── src/snapshot/matching.rs Forward sync matching algorithm. match_forward() pairs +│ InstanceSnapshots to existing RojoTree children using +│ recursive change-count scoring + greedy assignment. +│ Constants: UNMATCHED_PENALTY=10000, MAX_SCORING_DEPTH=3. Patch & Delivery (Server) ├── src/snapshot/patch_compute.rs compute_patch_set() diffs old snapshot vs tree → @@ -248,6 +270,16 @@ Patch Application (Plugin) │ via instanceMap). ├── plugin/src/Reconciler/diff.lua diff() compares virtual instance data against Studio. │ Used for confirmation dialog to show what will change. +├── plugin/src/Reconciler/matching.lua Recursive change-count scoring algorithm. matchChildren() +│ pairs virtual instances with Studio instances during +│ hydration. Signature: (virtualChildren, studioChildren, +│ virtualInstances). One of 3 parallel matching impls that +│ must produce identical pairings (see also src/snapshot/ +│ matching.rs and src/syncback/matching.rs). +├── plugin/src/Reconciler/trueEquals.lua Shared value equality. Fuzzy floats (epsilon 0.0001), +│ Color3 via RGB ints, CFrame/Vector3 component-wise, NaN +│ handling, nil/null-ref equivalence. Used by matching.lua +│ and diff.lua. ├── plugin/src/Reconciler/setProperty.lua setProperty() applies a single property to an instance. │ Handles special cases and error recovery. ├── plugin/src/Reconciler/getProperty.lua getProperty() reads a property from an instance. @@ -266,11 +298,20 @@ Snapshot System │ InstigatingSource (Path vs ProjectNode), SyncRule. ├── src/snapshot/tree.rs RojoTree: enhanced WeakDom with metadata. path_to_ids │ mapping, specified_id_to_refs tracking, get_instance_ -│ by_path() for path-based lookups. +│ by_path() for path-based lookups, filesystem_name_for() +│ for ref path resolution. +├── src/snapshot/matching.rs Forward sync matching: match_forward() pairs snapshot +│ children with existing tree children. Uses recursive +│ change-count scoring (same algorithm as syncback and +│ plugin matching). UNMATCHED_PENALTY=10000. ├── src/snapshot/patch.rs PatchSet, AppliedPatchSet, PatchAdd, PatchUpdate, │ AppliedPatchUpdate data structures. -├── src/snapshot/patch_compute.rs compute_patch_set(): diffs snapshot vs tree. -└── src/snapshot/patch_apply.rs apply_patch_set(): applies patch to tree. +├── src/snapshot/patch_compute.rs compute_patch_set(): diffs snapshot vs tree. Uses +│ match_forward() for child pairing. +└── src/snapshot/patch_apply.rs apply_patch_set(): applies patch to tree. Deferred ref + resolution (path-based Rojo_Ref_* and ID-based + Rojo_Target_*). Cleans internal attributes before + sending to plugin. Project System └── src/project.rs Project loading. Project, ProjectNode, PathNode types. @@ -293,9 +334,23 @@ rbx-dom (Roblox format libraries, git submodule) Plugin Shared Modules ├── plugin/src/InstanceMap.lua Instance ID ↔ Studio instance mapping. Signal management. +│ Also has onInstanceInserted callback for deferred Ref +│ resolution in ChangeBatcher. ├── plugin/src/PatchSet.lua PatchSet data structure and utilities. -├── plugin/src/Config.lua Protocol version, server version compatibility. +├── plugin/src/PatchTree.lua Builds tree for patch visualization. Accepts optional +│ gitMetadata to compute smart default selections (push/ +│ pull/nil) per instance based on git change status and +│ committed script hashes. +├── plugin/src/Config.lua Protocol version (6), server version compatibility, +│ defaultPort ("34873"). +├── plugin/src/Settings.lua Persistent settings (prefixed Atlas_ in Studio store). +│ Key defaults: oneShotSync=true, twoWaySync=true. ├── plugin/src/Types.lua Shared type definitions. +├── plugin/src/SHA1.luau SHA1 hashing. Computes git blob format hash ("blob +│ \0") for comparing script Source against +│ committed versions. Used by PatchTree for defaults. +├── plugin/src/XXH32.luau XXH32 hash function for plugin use. +├── plugin/src/ChangeMetadata.lua Change metadata tracking for sync operations. ├── plugin/src/strict.lua Module export wrapper (strict mode enforcement). └── plugin/src/DiffUtil.lua Diff utilities for comparing instance data. ``` @@ -309,8 +364,10 @@ When auditing a feature, read files in this order to build understanding before 3. **Syncback path:** `src/syncback/mod.rs`, then the relevant middleware's `syncback()` function -- understand how instances become filesystem 4. **Two-way sync server:** `src/web/api.rs` (`handle_api_write`, `syncback_*` functions), then `src/change_processor.rs` (`handle_tree_event`) -- understand how plugin writes are processed 5. **Two-way sync plugin:** `plugin/src/ChangeBatcher/` (encoding path), then `plugin/src/Reconciler/` (decoding path) -- understand both directions in the plugin -6. **File naming:** `src/syncback/file_names.rs` -- understand slugification, dedup, meta path computation -7. **Ref properties:** `src/syncback/ref_properties.rs`, `src/snapshot/patch_apply.rs` (deferred ref section) -- understand cross-instance references +6. **File naming:** `src/syncback/file_names.rs` -- understand slugification, dedup, meta path computation (`adjacent_meta_path` strips script suffixes: `Foo.server.luau` pairs with `Foo.meta.json5`) +7. **Ref properties:** `src/rojo_ref.rs` (constants, `RefPathIndex`, `ref_target_path_from_tree`), `src/syncback/ref_properties.rs` (syncback ref linking), `src/snapshot/patch_apply.rs` (deferred ref resolution) -- understand cross-instance references +8. **Matching algorithms:** `src/snapshot/matching.rs` (forward sync), `src/syncback/matching.rs` (CLI syncback), `plugin/src/Reconciler/matching.lua` (plugin) -- three parallel implementations that must produce identical pairings. Also `src/variant_eq.rs` (Rust) and `plugin/src/Reconciler/trueEquals.lua` (Lua) for property comparison +9. **Git integration:** `src/git.rs` (server-side git metadata, blob hashing, auto-staging), `plugin/src/PatchTree.lua` (default selection logic using gitMetadata), `plugin/src/SHA1.luau` (plugin-side hash computation) --- @@ -584,6 +641,32 @@ Any code path that writes to Studio instances without the user having explicitly - **Script type transitions**: ClassName changes (ModuleScript -> Script) cause file extension changes. Does the feature handle this? - **One-shot mode**: Does `Settings:get("oneShotSync")` correctly block the feature's outgoing writes? +#### 11l. Matching Algorithm Parity + +The matching algorithm exists in **3 parallel implementations** that must produce identical pairings for the same input: + +1. **Rust syncback** -- `src/syncback/matching.rs` (`match_children`, operates on `WeakDom` instances) +2. **Rust forward sync** -- `src/snapshot/matching.rs` (`match_forward`, operates on `InstanceSnapshot` vs `RojoTree`) +3. **Lua plugin** -- `plugin/src/Reconciler/matching.lua` (`Matching.matchChildren`, operates on virtual vs Studio instances) + +**Verify:** +- If the feature modifies matching logic in one implementation, was the same change applied to all three? +- Do all three use the same constants? (`UNMATCHED_PENALTY=10000`, `MAX_SCORING_DEPTH=3`) +- Do all three use the same grouping strategy? (fast-path by `(Name, ClassName)`, then recursive scoring for ambiguous groups) +- Is sort stability preserved? (Rust `sort_by` is stable; Lua `table.sort` is NOT -- ties must be broken by insertion index) +- Are the property comparison functions consistent? (`variant_eq` in Rust, `trueEquals.lua` in Lua -- both use fuzzy float equality, both treat nil and null-ref as equal) + +#### 11m. Git-Based Sync Defaults and Auto-Staging + +The confirmation dialog uses git metadata for smart default selections (`PatchTree.lua` + `src/git.rs`). Verify: + +- **Server-side `compute_git_metadata()`**: Does it correctly identify changed files vs HEAD? Does the two-phase tree lock (snapshot paths briefly, release before git subprocesses) prevent deadlocks? +- **Hash computation parity**: Server uses `compute_blob_sha1()` in `git.rs` and plugin uses `SHA1.luau` -- both must produce identical hashes for the same content using `SHA1("blob \0")` git blob format. +- **Default selection logic** (`PatchTree.lua`): File has no git changes → default "pull". File has git changes AND is script AND Studio Source matches committed hash → default "push". Otherwise → `nil` (user must decide). Verify these rules are applied correctly. +- **Auto-staging (`stageIds`)**: After confirmation, `api.rs` receives `stageIds` and calls `git_add`. Push-accepted items are always staged. Pull-accepted items are only staged if auto-selected (`defaultSelection ~= nil`). Manually-chosen pulls are left unstaged. Is this flow correct? +- **Staging split**: `api.rs` stages additions/removals/push files directly. `change_processor` stages Source writes after they complete (via `stage_ids` on PatchSet). Verify both paths handle the `stage_ids` list correctly. +- **No git repo**: When `git_repo_root` is `None`, all defaults should be `nil` and no staging should be attempted. + ### 12. Run Static Analysis Run `cargo clippy` on the modified code to catch Rust issues the audit might miss: