diff --git a/TODO.txt b/TODO.txt index 1ac45c66..01e57f2a 100644 --- a/TODO.txt +++ b/TODO.txt @@ -338,7 +338,3 @@ make sure this is applied to plugin-based syncback, too. [git] I notice we actually use the git cli in a lot of places. is there a git library we can use instead? - -[auto select studio/atlas] -when things get added AFTER the fact because we send the git history hashes on connect rather than continiously, it auto-selects Studio for things that are being edited in real-time in the sync dialog -how should we handle new files? because it could have been that someone just deleted it in studio. we can actually check if its a truly new file in Studio, like A but not staged! diff --git a/plugin/src/ApiContext.lua b/plugin/src/ApiContext.lua index 61496366..51035162 100644 --- a/plugin/src/ApiContext.lua +++ b/plugin/src/ApiContext.lua @@ -271,6 +271,12 @@ function ApiContext:write(patch, stageIds) end) end +function ApiContext:getGitMetadata() + local url = ("%s/api/git-metadata"):format(self.__baseUrl) + + return Http.get(url):andThen(rejectFailedRequests):andThen(Http.Response.msgpack) +end + function ApiContext:connectWebSocket(packetHandlers) local url = ("%s/api/socket/%s"):format(self.__baseUrl, self.__messageCursor) -- Convert HTTP/HTTPS URL to WS/WSS diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index 003042cb..fb3f693f 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -1000,6 +1000,9 @@ function App:startSession() return result end) + local pendingGitRefreshThread = nil + local GIT_METADATA_DEBOUNCE = 0.5 + serveSession:setPatchUpdateCallback(function(instanceMap, patch, changedIds) -- If all changes have been reverted, auto-accept the empty patch if PatchSet.isEmpty(patch) then @@ -1009,14 +1012,40 @@ function App:startSession() return end - -- Update the patchTree when new changes arrive during confirmation - -- The changedIds parameter contains IDs of items that were modified - -- so their selections can be reset (user must re-review) + -- Immediately rebuild with stale metadata so the UI reflects new items local gitMetadata = cachedServerInfo and cachedServerInfo.gitMetadata self:setState({ patchTree = PatchTree.build(patch, instanceMap, { "Property", "Current", "Incoming" }, gitMetadata), - changedIds = changedIds, -- Pass to UI so it can unselect changed items + changedIds = changedIds, }) + + -- Debounce: schedule a fresh git metadata fetch after rapid patches settle + if pendingGitRefreshThread then + task.cancel(pendingGitRefreshThread) + end + pendingGitRefreshThread = task.delay(GIT_METADATA_DEBOUNCE, function() + pendingGitRefreshThread = nil + apiContext + :getGitMetadata() + :andThen(function(freshMetadata) + if cachedServerInfo then + cachedServerInfo.gitMetadata = freshMetadata + end + if self.state.appStatus == AppStatus.Confirming then + self:setState({ + patchTree = PatchTree.build( + patch, + instanceMap, + { "Property", "Current", "Incoming" }, + freshMetadata + ), + }) + end + end) + :catch(function(err) + Log.warn("Failed to refresh git metadata: {}", tostring(err)) + end) + end) end) -- One-shot sync: disconnect after initial sync is fully complete (including any writes) diff --git a/plugin/src/PatchTree.lua b/plugin/src/PatchTree.lua index 9a6c5700..81bd7eeb 100644 --- a/plugin/src/PatchTree.lua +++ b/plugin/src/PatchTree.lua @@ -250,7 +250,7 @@ local PatchTree = {} -- Builds a new tree from a patch and instanceMap -- uses changeListHeaders in node.changeList --- gitMetadata is optional: { changedIds: {string}, scriptCommittedHashes: {[string]: {string}} } +-- gitMetadata is optional: { changedIds: {string}, scriptCommittedHashes: {[string]: {string}}, newFileIds: {string}? } function PatchTree.build(patch, instanceMap, changeListHeaders, gitMetadata) Timer.start("PatchTree.build") local clock = os.clock() @@ -267,6 +267,14 @@ function PatchTree.build(patch, instanceMap, changeListHeaders, gitMetadata) end end + local newFileIdSet = nil + if gitMetadata and gitMetadata.newFileIds then + newFileIdSet = {} + for _, id in gitMetadata.newFileIds do + newFileIdSet[id] = true + end + end + Timer.start("patch.updated") for _, change in patch.updated do clock = yieldIfNeeded(clock) @@ -518,8 +526,14 @@ function PatchTree.build(patch, instanceMap, changeListHeaders, gitMetadata) table.insert(changeList, 1, changeListHeaders) end - -- Add this node to tree - -- Default to nil (unselected) - user must explicitly choose Push/Pull/Skip + -- Truly new files (untracked in git) default to "push" (Atlas). + -- Tracked files appearing as additions are ambiguous (Studio may have + -- deleted them), so they remain nil for user review. + local defaultSelection = nil + if newFileIdSet and newFileIdSet[id] then + defaultSelection = "push" + end + tree:addNode(change.Parent, { id = change.Id, patchType = "Add", @@ -528,7 +542,7 @@ function PatchTree.build(patch, instanceMap, changeListHeaders, gitMetadata) changeInfo = changeInfo, changeList = changeList, instance = instanceMap.fromIds[id], - defaultSelection = nil, + defaultSelection = defaultSelection, }) end Timer.stop() diff --git a/src/git.rs b/src/git.rs index 24055b3a..e31a9caa 100644 --- a/src/git.rs +++ b/src/git.rs @@ -59,8 +59,23 @@ pub fn git_head_commit(repo_root: &Path) -> Option { } } -fn git_changed_files(repo_root: &Path, initial_head: Option<&str>) -> Option> { - let mut changed = HashSet::new(); +struct ChangedFiles { + tracked: HashSet, + untracked: HashSet, +} + +impl ChangedFiles { + fn all(&self) -> HashSet { + self.tracked.union(&self.untracked).cloned().collect() + } + + fn is_empty(&self) -> bool { + self.tracked.is_empty() && self.untracked.is_empty() + } +} + +fn git_changed_files(repo_root: &Path, initial_head: Option<&str>) -> Option { + let mut tracked = HashSet::new(); let diff_base = initial_head.unwrap_or("HEAD"); let diff_output = Command::new("git") @@ -78,7 +93,7 @@ fn git_changed_files(repo_root: &Path, initial_head: Option<&str>) -> Option) -> Option) -> Option = { let tree = tree_handle.lock().unwrap(); let mut result = Vec::new(); - for rel_path in &changed_files { + for rel_path in &all_changed { let abs_path = repo_root.join(rel_path); let canonical_path = std::fs::canonicalize(&abs_path).ok(); @@ -322,6 +343,11 @@ pub fn compute_git_metadata( }; // Lock released let changed_ids: Vec = resolved.iter().map(|ri| ri.id).collect(); + let new_file_ids: Vec = resolved + .iter() + .filter(|ri| untracked_set.contains(&ri.rel_path)) + .map(|ri| ri.id) + .collect(); let mut script_committed_hashes: HashMap> = HashMap::new(); #[derive(Clone, Copy)] @@ -393,6 +419,7 @@ pub fn compute_git_metadata( GitMetadata { changed_ids, script_committed_hashes, + new_file_ids, } } @@ -605,7 +632,8 @@ mod tests { fs::write(dir.path().join("script.luau"), "modified").unwrap(); let changed = git_changed_files(dir.path(), None).unwrap(); - assert!(changed.contains(&PathBuf::from("script.luau"))); + assert!(changed.tracked.contains(&PathBuf::from("script.luau"))); + assert!(changed.untracked.is_empty()); } #[test] @@ -618,7 +646,7 @@ mod tests { git_stage(dir.path(), "script.luau"); let changed = git_changed_files(dir.path(), None).unwrap(); - assert!(changed.contains(&PathBuf::from("script.luau"))); + assert!(changed.tracked.contains(&PathBuf::from("script.luau"))); } #[test] @@ -630,7 +658,8 @@ mod tests { fs::write(dir.path().join("new_file.luau"), "new").unwrap(); let changed = git_changed_files(dir.path(), None).unwrap(); - assert!(changed.contains(&PathBuf::from("new_file.luau"))); + assert!(changed.untracked.contains(&PathBuf::from("new_file.luau"))); + assert!(!changed.tracked.contains(&PathBuf::from("new_file.luau"))); } #[test] @@ -642,7 +671,7 @@ mod tests { fs::remove_file(dir.path().join("to_delete.luau")).unwrap(); let changed = git_changed_files(dir.path(), None).unwrap(); - assert!(changed.contains(&PathBuf::from("to_delete.luau"))); + assert!(changed.tracked.contains(&PathBuf::from("to_delete.luau"))); } #[test] @@ -657,9 +686,12 @@ mod tests { fs::write(dir.path().join("untracked.luau"), "brand new").unwrap(); let changed = git_changed_files(dir.path(), None).unwrap(); - assert!(changed.contains(&PathBuf::from("will_modify.luau"))); - assert!(changed.contains(&PathBuf::from("untracked.luau"))); - assert!(!changed.contains(&PathBuf::from("committed.luau"))); + let all = changed.all(); + assert!(all.contains(&PathBuf::from("will_modify.luau"))); + assert!(all.contains(&PathBuf::from("untracked.luau"))); + assert!(!all.contains(&PathBuf::from("committed.luau"))); + assert!(changed.tracked.contains(&PathBuf::from("will_modify.luau"))); + assert!(changed.untracked.contains(&PathBuf::from("untracked.luau"))); } #[test] @@ -670,7 +702,10 @@ mod tests { let changed = git_changed_files(dir.path(), None); assert!(changed.is_some()); - assert!(changed.unwrap().contains(&PathBuf::from("new.luau"))); + assert!(changed + .unwrap() + .untracked + .contains(&PathBuf::from("new.luau"))); } #[test] @@ -683,7 +718,7 @@ mod tests { fs::write(dir.path().join("src/module.luau"), "modified").unwrap(); let changed = git_changed_files(dir.path(), None).unwrap(); - assert!(changed.contains(&PathBuf::from("src/module.luau"))); + assert!(changed.tracked.contains(&PathBuf::from("src/module.luau"))); } #[test] @@ -700,13 +735,13 @@ mod tests { let without = git_changed_files(dir.path(), None).unwrap(); assert!( - !without.contains(&PathBuf::from("script.luau")), + !without.all().contains(&PathBuf::from("script.luau")), "without initial_head, committed file should NOT appear" ); let with = git_changed_files(dir.path(), Some(&initial_head)).unwrap(); assert!( - with.contains(&PathBuf::from("script.luau")), + with.tracked.contains(&PathBuf::from("script.luau")), "with initial_head, committed file should appear" ); } diff --git a/src/web/api.rs b/src/web/api.rs index 076df52d..7b77a7aa 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -160,6 +160,7 @@ pub async fn call( handle_api_syncback(request, &service, syncback_signal).await } (&Method::GET, "/api/validate-tree") => service.handle_api_validate_tree().await, + (&Method::GET, "/api/git-metadata") => service.handle_api_git_metadata().await, (_method, path) => msgpack( ErrorResponse::not_found(format!("Route not found: {}", path)), @@ -405,6 +406,18 @@ impl ApiService { }) } + async fn handle_api_git_metadata(&self) -> Response> { + let git_metadata = self.serve_session.git_repo_root().map(|repo_root| { + crate::git::compute_git_metadata( + &self.serve_session.tree_handle(), + repo_root, + self.serve_session.initial_head_commit(), + ) + }); + + msgpack_ok(&git_metadata) + } + /// Read-only tree freshness check for test infrastructure. /// Re-snapshots from disk and returns drift counts without applying corrections. async fn handle_api_validate_tree(&self) -> Response> { diff --git a/src/web/interface.rs b/src/web/interface.rs index 88244589..cd3e2647 100644 --- a/src/web/interface.rs +++ b/src/web/interface.rs @@ -214,6 +214,11 @@ pub struct GitMetadata { /// SHA1 hashes of prior versions (git blob format) for changed script instances. /// Contains 1-2 hashes per script: HEAD version and staged version (if different). pub script_committed_hashes: HashMap>, + /// Instance IDs whose backing files are untracked in git (truly new files, + /// never committed). Used by the plugin to default new-file additions to + /// "push" (Atlas) since they can't be "something Studio deleted." + #[serde(default)] + pub new_file_ids: Vec, } /// Response body from /api/rojo