Address code review feedback: improve error messages, clarify placeholders, fix validation logic

Co-authored-by: johndoe6345789 <224850594+johndoe6345789@users.noreply.github.com>

backend/operations.py

@@ -183,7 +183,7 @@ class OperationExecutor:
         key = ctx.interpolate(key_template)
         
         if if_absent and key in self.kv_store:
-            raise ValueError(f"Key {key} already exists")
+            raise ValueError(f"CAS operation failed: Key '{key}' already exists (if_absent constraint violated)")
         
         self.kv_store[key] = value
     
@@ -354,7 +354,14 @@ class OperationExecutor:
     # ========================================================================
     
     def proxy_fetch(self, ctx: ExecutionContext, args: Dict[str, Any]) -> None:
-        """Fetch from upstream proxy."""
+        """Fetch from upstream proxy.
+        
+        Note: This is a placeholder implementation. In production, this would:
+        1. Look up the upstream configuration from schema
+        2. Make an actual HTTP request with proper timeouts and retries
+        3. Handle authentication based on upstream.auth settings
+        4. Return the actual response
+        """
         upstream = args.get('upstream')
         method = args.get('method', 'GET')
         path_template = args.get('path')
@@ -362,8 +369,16 @@ class OperationExecutor:
         
         path = ctx.interpolate(path_template)
         
-        # In production, would look up upstream config and make actual request
-        # For now, return a mock response
+        # TODO: Implement actual proxy fetch with requests library
+        # upstream_config = get_upstream_config(upstream)
+        # response = requests.request(
+        #     method=method,
+        #     url=upstream_config['base_url'] + path,
+        #     timeout=(upstream_config['timeouts_ms']['connect']/1000, 
+        #              upstream_config['timeouts_ms']['read']/1000)
+        # )
+        
+        # Placeholder response for now
         response = {
             'status': 200,
             'body': None,

seed_data/load_seed_data.py

@@ -16,6 +16,8 @@ from typing import Dict, Any
 # Configuration
 BACKEND_URL = os.environ.get("BACKEND_URL", "http://localhost:5000")
 ADMIN_USERNAME = os.environ.get("ADMIN_USERNAME", "admin")
+# Security note: Default password should only be used for development/testing
+# In production, require explicit password via environment variable
 ADMIN_PASSWORD = os.environ.get("ADMIN_PASSWORD", "admin")
 
 

tests/validate_schema_compliance.py

@@ -35,10 +35,14 @@ def check_operation_coverage():
     implemented_ops = set()
     
     for name, method in inspect.getmembers(executor, predicate=inspect.ismethod):
-        if not name.startswith('_'):
-            # Convert method name back to operation name (e.g., kv_get -> kv.get)
-            op_name = name.replace('_', '.', 1)  # Only replace first underscore
-            implemented_ops.add(op_name)
+        if not name.startswith('_') and name != 'execute_pipeline':
+            # Convert method name back to operation name
+            # e.g., auth_require_scopes -> auth.require_scopes
+            # Replace first underscore with dot, keep rest as underscores
+            if '_' in name:
+                category, rest = name.split('_', 1)
+                op_name = f"{category}.{rest}"
+                implemented_ops.add(op_name)
     
     print(f"\nSchema defines {len(allowed_ops)} operations")
     print(f"Implementation provides {len(implemented_ops)} operations\n")