Fix/context config are attached to graphs

[amikofalvy]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
agents-manage-api	Ready	Preview	Comment	Oct 3, 2025 9:48pm
agents-manage-ui	Ready	Preview	Comment	Oct 3, 2025 9:48pm
agents-run-api	Ready	Preview	Comment	Oct 3, 2025 9:48pm

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
agents-docs	Skipped			Oct 3, 2025 9:48pm

[claude]

Claude finished @amikofalvy's task —— View job

---

PR Review: Context Config Graph Scoping

I've completed my comprehensive review of this architectural change moving context configurations from project-scoped to graph-scoped. This is a well-executed migration with proper data handling, but there are several critical design concerns that need to be addressed.

🎯 Summary

This PR successfully migrates context configs from (tenantId, projectId) to (tenantId, projectId, graphId) scoping with a robust SQL migration and comprehensive test coverage.

🔍 Key Strengths

✅ Excellent Migration Strategy: The SQL migration handles orphaned configs gracefully by excluding them rather than failing
✅ Comprehensive Test Coverage: 1000+ lines of integration tests covering edge cases, pagination, and data validation
✅ Data Integrity: Foreign key constraints ensure proper referential integrity
✅ Consistent API Patterns: Maintains existing CRUD patterns while updating scoping logic

⚠️ Critical Design Issues

1. Data Access Layer Inconsistency (High Priority)

The data access layer in contextConfigs.ts still uses project-level scoping instead of the new graph-level scoping:

// ❌ PROBLEM: Still using project scope instead of graph scope
export const getContextConfigById = (db: DatabaseClient) => 
  async (params: { scopes: ProjectScopeConfig; id: string }) => {
    return await db.query.contextConfigs.findFirst({
      where: and(
        eq(contextConfigs.tenantId, params.scopes.tenantId),
        eq(contextConfigs.projectId, params.scopes.projectId),
        // ❌ Missing: eq(contextConfigs.graphId, params.scopes.graphId),
        eq(contextConfigs.id, params.id)
      ),
    });
  };

Impact: This creates serious data access bugs where:

• Context configs from different graphs could be accidentally accessed
• Queries may return wrong results or fail to find configs
• Data isolation between graphs is compromised

Solution: Update all data access functions to require and use GraphScopeConfig instead of ProjectScopeConfig.

2. Breaking API Changes Without Version Strategy (High Priority)

The migration from project-scoped to graph-scoped APIs represents a breaking change:

Before: /tenants/{tenant}/projects/{project}/context-configs
After: /tenants/{tenant}/projects/{project}/graphs/{graph}/context-configs (implied)

Concerns:

• No backward compatibility strategy
• Existing client integrations will break
• No API versioning or deprecation path
• Tests still use old project-level endpoints

Recommendation: Consider implementing a migration strategy or API versioning to handle this transition.

3. Incomplete Schema Transition (Medium Priority)

The ContextConfigBuilder in ContextConfig.ts requires a graphId parameter but the API endpoints and data access layers aren't fully updated to support this architecture consistently.

4. Test Coverage Gap (Medium Priority)

While test coverage is comprehensive for CRUD operations, there are no tests specifically validating:

• Graph-level data isolation
• Cross-graph access prevention
• Foreign key constraint enforcement

🏗️ Architectural Considerations

Scoping Strategy Questions

1. Why graph-scoped vs agent-scoped? Context configs seem more naturally aligned with individual agents. Was this considered?

2. Multi-graph context sharing: If context configs are commonly shared across graphs in a project, this change reduces reusability. Is there a strategy for shared contexts?

3. Migration for existing users: How will existing deployments handle this breaking change?

Future Extensibility

The new graph-scoped approach is more restrictive. Consider whether you might need:

• Context config inheritance (project → graph → agent)
• Shared context libraries
• Cross-graph context references

🔧 Immediate Action Items

Must Fix (Blocking)

1. Update data access layer to use graph-scoped queries in all functions
2. Fix API endpoint patterns to be consistent with graph scoping
3. Update test helpers to create graph-scoped context configs properly

Should Consider

4. Add integration tests for graph-level data isolation
5. Document the breaking change and provide migration guidance
6. Consider adding validation for graph existence in context config operations

📝 Code Quality Notes

• Migration SQL: Well-structured with proper error handling
• Test Organization: Excellent organization with comprehensive edge cases
• Foreign Key Design: Proper cascade relationships implemented
• Data Validation: Good handling of null/empty values

🎯 Recommendation

This PR implements the graph-scoping migration correctly at the database level, but the data access layer and API consistency issues are critical blockers. The architectural change itself is sound, but the implementation needs to be completed across all layers before merging.

Priority: Address the data access layer inconsistencies first, as they represent potential data integrity and security issues.