audit: fix shared test context to minimise events sent to production relays

3 files changed, 119 insertions, 58 deletions

diff --git a/grasp-audit/src/bin/grasp-audit.rs b/grasp-audit/src/bin/grasp-audit.rs
index 29f9c9a..2aabefe 100644
--- a/grasp-audit/src/bin/grasp-audit.rs
+++ b/grasp-audit/src/bin/grasp-audit.rs
@@ -120,7 +120,27 @@ async fn main() -> Result<()> {
                 "all" => {
                     println!("Running all tests...\n");
                     let mut all_results = AuditResult::new("All GRASP-01 Tests");
-                    
+
+                    // Repository creation tests
+                    println!("  → Repository creation tests...");
+                    let repo_results = specs::RepositoryCreationTests::run_all(&client, &relay_domain).await;
+                    all_results.merge(repo_results);
+
+                    // Git clone tests
+                    println!("  → Git clone tests...");
+                    let clone_results = specs::GitCloneTests::run_all(&client, &relay_domain).await;
+                    all_results.merge(clone_results);
+
+                    // Push authorization tests
+                    println!("  → Push authorization tests...");
+                    let push_results = specs::PushAuthorizationTests::run_all(&client, &relay_domain).await;
+                    all_results.merge(push_results);
+
+                    // Event acceptance policy tests
+                    println!("  → Event acceptance policy tests...");
+                    let event_results = specs::EventAcceptancePolicyTests::run_all(&client).await;
+                    all_results.merge(event_results);
+
                     // NIP-01 smoke tests
                     println!("  → NIP-01 smoke tests...");
                     let nip01_results = specs::Nip01SmokeTests::run_all(&client).await;
@@ -131,30 +151,10 @@ async fn main() -> Result<()> {
                     let nip11_results = specs::Nip11DocumentTests::run_all(&client).await;
                     all_results.merge(nip11_results);
                     
-                    // Event acceptance policy tests
-                    println!("  → Event acceptance policy tests...");
-                    let event_results = specs::EventAcceptancePolicyTests::run_all(&client).await;
-                    all_results.merge(event_results);
-
                     // CORS tests
                     println!("  → CORS tests...");
                     let cors_results = specs::CorsTests::run_all(&client, &relay_domain).await;
                     all_results.merge(cors_results);
-
-                    // Git clone tests
-                    println!("  → Git clone tests...");
-                    let clone_results = specs::GitCloneTests::run_all(&client, &relay_domain).await;
-                    all_results.merge(clone_results);
-
-                    // Push authorization tests
-                    println!("  → Push authorization tests...");
-                    let push_results = specs::PushAuthorizationTests::run_all(&client, &relay_domain).await;
-                    all_results.merge(push_results);
-
-                    // Repository creation tests
-                    println!("  → Repository creation tests...");
-                    let repo_results = specs::RepositoryCreationTests::run_all(&client, &relay_domain).await;
-                    all_results.merge(repo_results);
                     
                     println!();
                     all_results
diff --git a/grasp-audit/src/client.rs b/grasp-audit/src/client.rs
index 8b96f4f..4f3401c 100644
--- a/grasp-audit/src/client.rs
+++ b/grasp-audit/src/client.rs
@@ -1,11 +1,22 @@
 //! Audit client for testing GRASP implementations
 
 use crate::audit::{AuditConfig, AuditEventBuilder, AuditMode};
+use crate::fixtures::FixtureKind;
 use anyhow::{anyhow, Result};
 use nostr_sdk::prelude::*;
+use std::collections::HashMap;
+use std::sync::{Arc, Mutex};
 use std::time::Duration;
 
+/// Type alias for the fixture cache - shared across TestContext instances
+pub type FixtureCache = Arc<Mutex<HashMap<FixtureKind, Event>>>;
+
 /// Client for auditing GRASP implementations
+///
+/// The AuditClient owns a fixture cache that is shared across all TestContext
+/// instances created from this client. This provides natural cache sharing:
+/// - CLI creates one AuditClient → fixtures shared across all tests
+/// - cargo test creates one AuditClient per test → fixtures isolated per test
 pub struct AuditClient {
     client: Client,
     pub config: AuditConfig,
@@ -14,6 +25,8 @@ pub struct AuditClient {
     maintainer_keys: Keys,
     /// Recursive maintainer keys for testing recursive authorization scenarios
     recursive_maintainer_keys: Keys,
+    /// Fixture cache for TestContext instances - shared across all contexts using this client
+    fixture_cache: FixtureCache,
 }
 
 impl AuditClient {
@@ -30,6 +43,7 @@ impl AuditClient {
             keys,
             maintainer_keys,
             recursive_maintainer_keys,
+            fixture_cache: Arc::new(Mutex::new(HashMap::new())),
         }
     }
 
@@ -88,9 +102,19 @@ impl AuditClient {
             keys,
             maintainer_keys,
             recursive_maintainer_keys,
+            fixture_cache: Arc::new(Mutex::new(HashMap::new())),
         })
     }
 
+    /// Get the fixture cache for TestContext usage
+    ///
+    /// This cache is shared across all TestContext instances created from this client.
+    /// In CLI mode (one client for all tests), fixtures are reused.
+    /// In test mode (one client per test), fixtures are isolated.
+    pub fn fixture_cache(&self) -> &FixtureCache {
+        &self.fixture_cache
+    }
+
     /// Get the public key for this audit client
     pub fn public_key(&self) -> PublicKey {
         self.keys.public_key()
@@ -488,6 +512,7 @@ mod tests {
             keys: keys.clone(),
             maintainer_keys,
             recursive_maintainer_keys,
+            fixture_cache: Arc::new(Mutex::new(HashMap::new())),
         };
 
         let _builder = client.event_builder(Kind::TextNote, "test content");
@@ -508,6 +533,7 @@ mod tests {
             keys: keys.clone(),
             maintainer_keys,
             recursive_maintainer_keys,
+            fixture_cache: Arc::new(Mutex::new(HashMap::new())),
         };
 
         // Create an event with a custom tag
diff --git a/grasp-audit/src/fixtures.rs b/grasp-audit/src/fixtures.rs
index 89531c6..81620f6 100644
--- a/grasp-audit/src/fixtures.rs
+++ b/grasp-audit/src/fixtures.rs
@@ -6,6 +6,17 @@
 //! - **CI Mode (Isolated)**: Creates fresh events for each test, ensuring complete isolation
 //! - **Production Mode (Shared)**: Reuses shared fixtures to minimize event publication
 //!
+//! # Cache Sharing Strategy
+//!
+//! The fixture cache lives on the `AuditClient`, not on `TestContext`. This provides
+//! natural cache sharing semantics:
+//!
+//! - **CLI mode**: Creates one `AuditClient` → fixtures shared across all tests
+//! - **cargo test**: Creates one `AuditClient` per test → fixtures isolated per test
+//!
+//! This eliminates the need for global state while still enabling fixture reuse
+//! when appropriate.
+//!
 //! # Example
 //!
 //! ```no_run
@@ -25,8 +36,6 @@
 use crate::{AuditClient, AuditMode};
 use anyhow::{Context, Result};
 use nostr_sdk::prelude::Event;
-use std::collections::HashMap;
-use std::sync::{Arc, Mutex};
 
 /// Deterministic commit hash used in RepoState fixtures (Owner variant)
 /// This is the hash produced by creating a commit with:
@@ -161,6 +170,13 @@ impl From<AuditMode> for ContextMode {
 /// - In Isolated mode: Creates fresh events for each test
 /// - In Shared mode: Caches and reuses events across tests
 ///
+/// # Cache Location
+///
+/// The fixture cache lives on `AuditClient`, not on `TestContext`. This means:
+/// - Multiple `TestContext` instances from the same client share the cache
+/// - CLI mode (one client) naturally shares fixtures across all tests
+/// - Test mode (one client per test) naturally isolates fixtures
+///
 /// # Example
 ///
 /// ```no_run
@@ -181,20 +197,18 @@ impl From<AuditMode> for ContextMode {
 pub struct TestContext<'a> {
     client: &'a AuditClient,
     mode: ContextMode,
-    cache: Arc<Mutex<HashMap<FixtureKind, Event>>>,
 }
 
 impl<'a> TestContext<'a> {
     /// Create a new test context
     ///
     /// The context mode is automatically determined from the client's audit config.
+    /// The fixture cache is borrowed from the client, enabling natural sharing:
+    /// - Same client = shared cache (CLI mode behavior)
+    /// - Different clients = isolated caches (test mode behavior)
     pub fn new(client: &'a AuditClient) -> Self {
         let mode = ContextMode::from(client.config.mode);
-        Self {
-            client,
-            mode,
-            cache: Arc::new(Mutex::new(HashMap::new())),
-        }
+        Self { client, mode }
     }
 
     /// Create a test context with explicit mode override
@@ -202,11 +216,7 @@ impl<'a> TestContext<'a> {
     /// This is useful for testing the context itself or for advanced use cases
     /// where you want to override the default mode behavior.
     pub fn with_mode(client: &'a AuditClient, mode: ContextMode) -> Self {
-        Self {
-            client,
-            mode,
-            cache: Arc::new(Mutex::new(HashMap::new())),
-        }
+        Self { client, mode }
     }
 
     /// Get a fixture, creating it if needed based on mode
@@ -261,15 +271,28 @@ impl<'a> TestContext<'a> {
     }
 
     /// Get or create a shared fixture (caches for reuse)
+    ///
+    /// Uses the client's fixture cache to ensure fixtures are reused across
+    /// all TestContext instances in Production mode.
     async fn get_or_create_shared(&self, kind: FixtureKind) -> Result<Event> {
-        // Check cache first
+        // Check client's cache first (shared across all TestContext instances using same client)
         {
-            let cache = self.cache.lock().unwrap();
+            let cache = self.client.fixture_cache().lock().unwrap();
             if let Some(event) = cache.get(&kind) {
+                tracing::debug!("get_or_create_shared({:?}) found in client cache", kind);
                 return Ok(event.clone());
             }
         }
 
+        // Check relay connection before attempting to build
+        let is_connected = self.client.is_connected().await;
+        if !is_connected {
+            return Err(anyhow::anyhow!(
+                "Relay connection lost before building {:?} fixture (shared cache mode)",
+                kind
+            ));
+        }
+
         // Not in cache, create it
         let event = self
             .build_fixture(kind)
@@ -286,64 +309,76 @@ impl<'a> TestContext<'a> {
                 )
             })?;
 
-        // Store in cache
+        // Store in client's cache (shared across all TestContext instances using same client)
         {
-            let mut cache = self.cache.lock().unwrap();
+            let mut cache = self.client.fixture_cache().lock().unwrap();
             cache.insert(kind, event.clone());
+            tracing::debug!("get_or_create_shared({:?}) stored in client cache ({} entries)", kind, cache.len());
         }
 
         Ok(event)
     }
 
-    /// Get or create a ValidRepo, with caching within the TestContext.
+    /// Get or create a ValidRepo, with caching.
     /// This is a helper method that avoids async recursion by not going
     /// through get_fixture. It handles the repo specifically.
     ///
-    /// IMPORTANT: We always cache within a TestContext instance to ensure
-    /// fixture dependencies work correctly. The isolation between tests
-    /// comes from each test having its own TestContext with a fresh cache.
+    /// Uses client's fixture_cache for caching - in Shared mode this enables
+    /// cross-test reuse when the same client is used.
     async fn get_or_create_repo(&self) -> Result<Event> {
-        // Always check cache first - this ensures fixture dependencies work
-        // (e.g., MaintainerRepoAndState needs the SAME repo_id as RepoState)
+        // Check client's cache first (shared across all TestContext instances using same client)
         {
-            let cache = self.cache.lock().unwrap();
+            let cache = self.client.fixture_cache().lock().unwrap();
             if let Some(event) = cache.get(&FixtureKind::ValidRepo) {
+                tracing::debug!("get_or_create_repo() found in client cache");
                 return Ok(event.clone());
             }
         }
 
+        // Check relay connection before creating repo
+        let is_connected = self.client.is_connected().await;
+        if !is_connected {
+            return Err(anyhow::anyhow!(
+                "Relay connection lost before creating ValidRepo fixture"
+            ));
+        }
+
         // Create a new repo
         let test_name = format!(
             "fixture-{:?}-{}",
             FixtureKind::ValidRepo,
             &uuid::Uuid::new_v4().to_string()[..8]
         );
-        let repo = self.client.create_repo_announcement(&test_name).await?;
+        
+        let repo = self.client.create_repo_announcement(&test_name).await
+            .with_context(|| format!("create_repo_announcement failed for {}", test_name))?;
 
         // Send it
-        self.client.send_event(repo.clone()).await?;
+        self.client.send_event(repo.clone()).await
+            .with_context(|| "Failed to send repo announcement to relay")?;
 
-        // Always cache it - isolation comes from each test having its own TestContext
+        // Store in client's cache (shared across all TestContext instances using same client)
         {
-            let mut cache = self.cache.lock().unwrap();
+            let mut cache = self.client.fixture_cache().lock().unwrap();
             cache.insert(FixtureKind::ValidRepo, repo.clone());
+            tracing::debug!("get_or_create_repo() stored in client cache ({} entries)", cache.len());
         }
 
         Ok(repo)
     }
 
-    /// Get or create a RepoWithIssue, with caching within the TestContext.
+    /// Get or create a RepoWithIssue, with caching via the client.
     /// Returns the issue event (repo is already sent/cached via get_or_create_repo).
     async fn get_or_create_issue(&self) -> Result<Event> {
-        // Always check cache first - ensures fixture dependencies work
+        // Check client's cache first
         {
-            let cache = self.cache.lock().unwrap();
+            let cache = self.client.fixture_cache().lock().unwrap();
             if let Some(event) = cache.get(&FixtureKind::RepoWithIssue) {
                 return Ok(event.clone());
             }
         }
 
-        // Get or create repo (reuses cached within this TestContext)
+        // Get or create repo (reuses cached via client)
         let repo = self.get_or_create_repo().await?;
 
         // Create the issue
@@ -357,9 +392,9 @@ impl<'a> TestContext<'a> {
         // Send it
         self.client.send_event(issue.clone()).await?;
 
-        // Always cache it - isolation comes from each test having its own TestContext
+        // Store in client's cache
         {
-            let mut cache = self.cache.lock().unwrap();
+            let mut cache = self.client.fixture_cache().lock().unwrap();
             cache.insert(FixtureKind::RepoWithIssue, issue.clone());
         }
 
@@ -707,10 +742,10 @@ impl<'a> TestContext<'a> {
 
     /// Clear the fixture cache
     ///
-    /// This is useful for tests that want to ensure fresh fixtures
-    /// even in shared mode.
+    /// This clears the client's fixture cache, affecting all TestContext
+    /// instances using the same client.
     pub fn clear_cache(&self) {
-        let mut cache = self.cache.lock().unwrap();
+        let mut cache = self.client.fixture_cache().lock().unwrap();
         cache.clear();
     }
 }