fix(#469 review): skip caching workspace_entry results for removed workspaces

Address rchiodo's verified review comment. The generation guard alone
catches the race where configure() runs *during* a parse, but misses
the case where configure() removes a workspace *before* workspace_entry
snapshots the generation:

  T1: try_from snapshots state.workspaces = [W, X]
  T2: configure removes W, generation = N+1
  T1: workspace_entry(W) misses cache, snapshots generation = N+1
  T1: parses W, re-acquires lock, generation still matches, inserts W

W now lives in state.parsed as an orphan until the next configure()
clears it.

Add a membership check before insert: if the workspace is no longer in
state.workspaces, return the parsed value (so the in-flight caller can
finish) without caching it. Impact of the prior behaviour was low (cache
gets cleared on next configure and new callers never re-iterate the
removed workspace), but the membership check keeps the cache invariant
clean: parsed only contains entries for currently configured workspaces.

Test: workspace_entry_does_not_cache_for_unconfigured_workspace pins the
contract directly without needing threads.

Author: eleanorjboyd

crates/pet-hatch/src/lib.rs

@@ -141,12 +141,19 @@ impl Hatch {
     /// performed outside the state mutex so concurrent `try_from()` calls
     /// for *other* workspaces are not blocked by a slow filesystem.
     ///
-    /// Race handling: `configure()` may run between our cache-miss check
-    /// and our re-acquire to insert, invalidating the cache while we were
-    /// parsing. We snapshot the generation before the parse and only
-    /// insert if the generation has not changed; otherwise we discard the
-    /// stale parse and retry. The loop is bounded in practice because
-    /// `configure()` is a client-driven, infrequent operation.
+    /// Race handling:
+    /// * `configure()` may run between our cache-miss check and our
+    ///   re-acquire to insert, invalidating the cache while we were
+    ///   parsing. We snapshot the generation before the parse and discard
+    ///   the result if the generation moved, then retry against the
+    ///   current state.
+    /// * `configure()` may also have removed `workspace` from the
+    ///   configured set before we were called (the caller iterates a
+    ///   snapshot of `state.workspaces` taken before `workspace_entry`).
+    ///   In that case we still return the parsed value so the in-flight
+    ///   caller can complete, but we do not cache it — otherwise the
+    ///   cache would hold an orphan entry for a workspace that is no
+    ///   longer configured until the next `configure()` clears it.
     fn workspace_entry(&self, workspace: &Path) -> Arc<WorkspaceEntry> {
         loop {
             // Fast path: cache hit. Also snapshot the generation so we can
@@ -175,6 +182,15 @@ impl Hatch {
                 // Drop it and retry against the current generation.
                 continue;
             }
+            if !state.workspaces.iter().any(|w| w == workspace) {
+                // `workspace` is not in the current configured set (the
+                // caller's snapshot was taken before a configure() that
+                // removed it). Return the parsed result so the in-flight
+                // caller can finish without re-reading disk, but don't
+                // pollute `parsed` with an orphan entry that would
+                // outlive the workspace until the next configure().
+                return parsed;
+            }
             // A concurrent caller for the same workspace may have already
             // inserted while we were parsing; prefer the existing entry
             // so every caller observes the same `Arc`. `or_insert_with`
@@ -1448,6 +1464,54 @@ mod tests {
         );
     }
 
+    #[test]
+    fn workspace_entry_does_not_cache_for_unconfigured_workspace() {
+        // Race scenario: try_from() / find() snapshots state.workspaces,
+        // then configure() removes a workspace before workspace_entry()
+        // re-acquires the lock to insert. Without a workspaces-membership
+        // check, the parsed cache would hold an orphan entry for a
+        // workspace that is no longer configured, persisting until the
+        // next configure(). The generation guard alone is not enough
+        // here because configure() can land *before* workspace_entry()
+        // snapshots the generation.
+        //
+        // Verify the contract directly: calling workspace_entry() for a
+        // workspace not in state.workspaces returns a usable parsed
+        // value but does NOT populate the cache.
+        let temp = TempDir::new().unwrap();
+        let configured = temp.path().join("configured");
+        let removed = temp.path().join("removed");
+        fs::create_dir_all(&configured).unwrap();
+        fs::create_dir_all(&removed).unwrap();
+        fs::write(
+            removed.join("pyproject.toml"),
+            b"[tool.hatch.dirs.env]\nvirtual = \".hatch\"\n",
+        )
+        .unwrap();
+
+        let locator = make_locator(None);
+        let config = Configuration {
+            workspace_directories: Some(vec![configured.clone()]),
+            ..Configuration::default()
+        };
+        locator.configure(&config);
+
+        // `removed` is not in state.workspaces. Calling workspace_entry
+        // for it must still return a parsed entry (the in-flight caller
+        // needs a usable value) but must not pollute the cache.
+        let entry = locator.workspace_entry(&removed);
+        assert_eq!(entry.virtual_dirs, vec![norm_case(&removed.join(".hatch"))]);
+        let state = locator.state.lock().unwrap();
+        assert!(
+            !state.parsed.contains_key(&removed),
+            "parsed cache must not contain entries for unconfigured workspaces"
+        );
+        assert!(
+            !state.parsed.contains_key(&configured),
+            "configured workspace was never accessed; cache should be empty"
+        );
+    }
+
     #[cfg(target_os = "linux")]
     #[test]
     fn data_dir_uses_xdg_data_home_when_set() {