Add Conductor workspace configuration #1040
Conversation
This adds Conductor integration to automate workspace setup and testing. Configuration includes: - Setup script that runs automatically when creating new workspaces - Automated dependency installation using uv with frozen lock file - Environment file linking from root repository - Workspace-specific Makefile using virtualenv directly - Run script for executing the test suite via "Run" button The setup script: - Verifies prerequisites (uv, Python 3.10+) - Copies necessary files and creates symlinks to source code - Installs all 185 development dependencies - Links .env file and validates OPENAI_API_KEY - Provides helpful error messages for missing prerequisites 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
danielchalef
had a problem deploying
to
development
GitHub Actions
Failure
danielchalef
had a problem deploying
to
development
GitHub Actions
Failure
Critical Issues with PR ScopeThis PR has significant scope creep that needs to be addressed before merging: 1. Unrelated Files IncludedSUMMARY_PIPELINE_STATE.md - This is a 126-line design document about node summary pipeline optimizations that has nothing to do with Conductor integration. This appears to be accidentally committed and should be removed or moved to a separate PR. .cursor/worktrees.json - This is Cursor IDE-specific configuration that should not be committed to the repository. This is IDE-specific and should be in .gitignore. 2. Major Unrelated Feature: FalkorDB Lite SupportThe PR includes a substantial new feature adding FalkorDB Lite support (69 lines in falkordb_driver.py) that is completely unrelated to Conductor workspace configuration:
This feature should be in a separate PR with proper review focus on the FalkorDB integration. 3. PR Title MismatchThe PR is titled "Add Conductor workspace configuration" but actually includes 3-4 distinct changes that should be separate PRs. RecommendationSplit this into multiple PRs:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| fi | ||
|
|
||
| # Check Python version | ||
| python_version=$(python3 --version 2>&1 | awk '{print $2}') |
There was a problem hiding this comment.
The required_version variable is set but never used. Either remove it or use it in a more informative error message like:
echo "❌ Error: Python ${required_version}+ is required (found: $python_version)"
|
|
||
| # Copy necessary files from root repo | ||
| echo "📄 Copying project files from root repo..." | ||
| cp "$CONDUCTOR_ROOT_PATH/pyproject.toml" . |
There was a problem hiding this comment.
Missing validation that $CONDUCTOR_ROOT_PATH is set. The script will fail with confusing errors if this environment variable is not defined. Add at the start of the script:
if [ -z "$CONDUCTOR_ROOT_PATH" ]; then
echo "❌ Error: CONDUCTOR_ROOT_PATH environment variable is not set"
exit 1
fi
|
|
||
| # Create symlink to source code instead of copying | ||
| echo "🔗 Creating symlinks to source code..." | ||
| ln -sf "$CONDUCTOR_ROOT_PATH/graphiti_core" graphiti_core |
There was a problem hiding this comment.
Using ln -sf will silently overwrite existing symlinks. If a workspace is re-setup, this could cause issues if the symlinks already exist. Consider checking first or adding validation that the symlinks point to the expected location.
| # Internal helpers | ||
| # ----------------- | ||
|
|
||
| async def _await_graph_query(graph: Any, cypher: str, params: dict[str, Any] | None): |
There was a problem hiding this comment.
This function calls query_callable twice when the result is not awaitable - once on line 365 to check if it's awaitable, and again on line 369 to actually run it. This executes the query twice for sync clients.
Fix:
async def _await_graph_query(graph: Any, cypher: str, params: dict[str, Any] | None):
query_callable = getattr(graph, 'query')
result = query_callable(cypher, params)
if inspect.isawaitable(result):
return await result
# Result already executed for sync path, just return it
return result
| # Lazy import to avoid mandatory dependency when not using Lite | ||
| try: | ||
| from redislite.falkordb_client import FalkorDB as LiteFalkorDB # type: ignore | ||
| except Exception as e: # broad to surface helpful message |
There was a problem hiding this comment.
Catching bare Exception is too broad. This will catch KeyboardInterrupt, SystemExit, and other exceptions that should propagate. Use ImportError specifically since that's what you're trying to catch.
| ) from e | ||
|
|
||
| db_path = lite_db_path or '/tmp/falkordb.db' | ||
| lite_client = LiteFalkorDB(db_path) |
There was a problem hiding this comment.
Hardcoding /tmp/falkordb.db as the default is problematic:
/tmpmay be cleared on reboot, losing data- No write permission guarantees in
/tmpon all systems - No isolation between different processes/users
Consider using a more appropriate default like ~/.local/share/graphiti/falkordb.db or require the user to specify the path explicitly.
| db_path = lite_db_path or '/tmp/falkordb.db' | ||
| lite_client = LiteFalkorDB(db_path) | ||
| self.client = _AsyncLiteClientAdapter(lite_client) | ||
| self._is_lite = True |
There was a problem hiding this comment.
The _is_lite instance variable is set but never used anywhere in the class. Either remove it or document why it's needed for future functionality.
| from graphiti_core.driver.falkordb_driver import FalkorDriver as _FD | ||
|
|
||
| try: | ||
| _FD(lite=True, database='default_db') |
There was a problem hiding this comment.
This test only verifies that initialization doesn't raise an exception, but doesn't actually validate:
- That the lite client adapter is used
- That the adapter works correctly
- That queries can be executed
The test should verify the actual behavior, not just that it doesn't crash. For example:
driver = _FD(lite=True, database='default_db')
assert isinstance(driver.client, _AsyncLiteClientAdapter)
Summary
Adds Conductor integration to automate workspace setup and testing, enabling parallel development workflows across multiple isolated workspaces.
Changes
Features
Automated Setup Script
Run Script
make testBenefits
Testing
Once merged, all Conductor workspaces will automatically have access to these scripts.
🤖 Generated with Claude Code