feat: implement PlatformWorkspaceContext for GAP-3#1380
Conversation
- Add PlatformWorkspaceContext class implementing WorkspaceContextProtocol - Provides workspace context and agent config via database queries - Scopes agent queries to workspace for security - Returns None for missing workspace/agent as per spec - Export PlatformWorkspaceContext from services module 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
Greptile SummaryThis PR adds Key observations:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Consumer
participant PlatformWorkspaceContext
participant AsyncSession
participant DB as Database
Consumer->>PlatformWorkspaceContext: __init__(workspace_id, session)
Note over PlatformWorkspaceContext: stores workspace_id + session
Consumer->>PlatformWorkspaceContext: get_workspace_context(workspace_id)
PlatformWorkspaceContext->>PlatformWorkspaceContext: "guard: workspace_id == self.workspace_id?"
alt mismatched workspace_id
PlatformWorkspaceContext-->>Consumer: None
else matched
PlatformWorkspaceContext->>AsyncSession: session.get(Workspace, workspace_id)
AsyncSession->>DB: "SELECT * FROM workspace WHERE id=?"
DB-->>AsyncSession: Workspace row / None
AsyncSession-->>PlatformWorkspaceContext: workspace obj / None
alt not found
PlatformWorkspaceContext-->>Consumer: None
else found
PlatformWorkspaceContext-->>Consumer: "json.dumps({id, name, slug, description, settings})"
end
end
Consumer->>PlatformWorkspaceContext: get_agent_config(workspace_id, agent_id)
PlatformWorkspaceContext->>PlatformWorkspaceContext: "guard: workspace_id == self.workspace_id?"
alt mismatched workspace_id
PlatformWorkspaceContext-->>Consumer: None
else matched
PlatformWorkspaceContext->>AsyncSession: "execute(SELECT Agent WHERE id=? AND workspace_id=?)"
AsyncSession->>DB: scoped agent query
DB-->>AsyncSession: Agent row / None
AsyncSession-->>PlatformWorkspaceContext: agent obj / None
alt not found
PlatformWorkspaceContext-->>Consumer: None
else found
PlatformWorkspaceContext-->>Consumer: "Dict{system_prompt, model, tools, max_concurrent_tasks, ...}"
end
end
Reviews (3): Last reviewed commit: "fix(platform): align PlatformWorkspaceCo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/praisonai-platform/praisonai_platform/services/workspace_context.py
Outdated
Show resolved
Hide resolved
src/praisonai-platform/praisonai_platform/services/workspace_context.py
Outdated
Show resolved
Hide resolved
src/praisonai-platform/praisonai_platform/services/workspace_context.py
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/praisonai-platform/praisonai_platform/services/workspace_context.py (1)
48-54: Return defensive copies for mutable JSON fields.
workspace.settingsandagent.runtime_configare returned directly. If callers mutate them, ORM state can be mutated unintentionally. Return copies instead.Proposed defensive-copy patch
return { "id": workspace.id, "name": workspace.name, "slug": workspace.slug, "description": workspace.description, - "settings": workspace.settings or {}, + "settings": dict(workspace.settings or {}), } ... return { "id": agent.id, "name": agent.name, "runtime_mode": agent.runtime_mode, "instructions": agent.instructions, - "config": agent.runtime_config or {}, + "config": dict(agent.runtime_config or {}), "max_concurrent_tasks": agent.max_concurrent_tasks, }Also applies to: 79-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/praisonai-platform/praisonai_platform/services/workspace_context.py` around lines 48 - 54, The returned dicts expose mutable JSON fields directly (workspace.settings and agent.runtime_config); make defensive copies before returning to avoid accidental ORM state mutation — e.g., import copy and replace direct returns of workspace.settings and agent.runtime_config with copy.deepcopy(workspace.settings or {}) and copy.deepcopy(agent.runtime_config or {}), and apply the same change in both return sites (the block returning workspace fields and the block returning agent fields) so callers receive independent copies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/praisonai-platform/praisonai_platform/services/workspace_context.py`:
- Around line 36-86: The implementations of get_workspace_context and
get_agent_config do not match WorkspaceContextProtocol: both must accept a
workspace_id parameter (caller-provided) and get_workspace_context must return
Optional[str] per the protocol instead of a dict; update the signatures of
get_workspace_context(self, workspace_id: str) -> Optional[str] and
get_agent_config(self, agent_id: str, workspace_id: str) -> Optional[Dict[str,
Any]] (or whatever the protocol specifies for agent config), use the passed
workspace_id in the queries instead of self.workspace_id, and change
get_workspace_context to return the workspace_id (or None) rather than a dict so
the class conforms to WorkspaceContextProtocol; update any consumers to pass
workspace_id accordingly.
---
Nitpick comments:
In `@src/praisonai-platform/praisonai_platform/services/workspace_context.py`:
- Around line 48-54: The returned dicts expose mutable JSON fields directly
(workspace.settings and agent.runtime_config); make defensive copies before
returning to avoid accidental ORM state mutation — e.g., import copy and replace
direct returns of workspace.settings and agent.runtime_config with
copy.deepcopy(workspace.settings or {}) and copy.deepcopy(agent.runtime_config
or {}), and apply the same change in both return sites (the block returning
workspace fields and the block returning agent fields) so callers receive
independent copies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d7b6eb4-4cbd-4c24-bf85-96752eb08763
📒 Files selected for processing (2)
src/praisonai-platform/praisonai_platform/services/__init__.pysrc/praisonai-platform/praisonai_platform/services/workspace_context.py
| async def get_workspace_context(self) -> Optional[Dict[str, Any]]: | ||
| """ | ||
| Get workspace-level context for agents. | ||
| Returns: | ||
| Dict containing workspace data if found, None otherwise. | ||
| Keys: id, name, slug, description, settings | ||
| """ | ||
| workspace = await self._session.get(Workspace, self.workspace_id) | ||
| if workspace is None: | ||
| return None | ||
|
|
||
| return { | ||
| "id": workspace.id, | ||
| "name": workspace.name, | ||
| "slug": workspace.slug, | ||
| "description": workspace.description, | ||
| "settings": workspace.settings or {}, | ||
| } | ||
|
|
||
| async def get_agent_config(self, agent_id: str) -> Optional[Dict[str, Any]]: | ||
| """ | ||
| Get agent configuration from the platform. | ||
| Args: | ||
| agent_id: The agent identifier | ||
| Returns: | ||
| Agent configuration dict if found, None otherwise. | ||
| Keys: id, name, runtime_mode, instructions, config, max_concurrent_tasks | ||
| """ | ||
| # Query for agent scoped to the workspace | ||
| stmt = ( | ||
| select(Agent) | ||
| .where(Agent.id == agent_id) | ||
| .where(Agent.workspace_id == self.workspace_id) | ||
| ) | ||
| result = await self._session.execute(stmt) | ||
| agent = result.scalar_one_or_none() | ||
|
|
||
| if agent is None: | ||
| return None | ||
|
|
||
| return { | ||
| "id": agent.id, | ||
| "name": agent.name, | ||
| "runtime_mode": agent.runtime_mode, | ||
| "instructions": agent.instructions, | ||
| "config": agent.runtime_config or {}, | ||
| "max_concurrent_tasks": agent.max_concurrent_tasks, | ||
| } No newline at end of file |
There was a problem hiding this comment.
Protocol contract is currently incompatible with WorkspaceContextProtocol.
Line 36 and Line 56 change the callable shape and return contract (workspace_id missing from both methods; workspace context returns dict instead of protocol’s Optional[str]). This breaks substitutability for any caller typed against the protocol.
Please align this class to the protocol contract, or update the protocol + all consumers in the same change set.
Proposed contract-alignment skeleton
- async def get_workspace_context(self) -> Optional[Dict[str, Any]]:
+ async def get_workspace_context(self, workspace_id: str) -> Optional[str]:
...
- async def get_agent_config(self, agent_id: str) -> Optional[Dict[str, Any]]:
+ async def get_agent_config(self, workspace_id: str, agent_id: str) -> Optional[Dict[str, Any]]:
...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/praisonai-platform/praisonai_platform/services/workspace_context.py`
around lines 36 - 86, The implementations of get_workspace_context and
get_agent_config do not match WorkspaceContextProtocol: both must accept a
workspace_id parameter (caller-provided) and get_workspace_context must return
Optional[str] per the protocol instead of a dict; update the signatures of
get_workspace_context(self, workspace_id: str) -> Optional[str] and
get_agent_config(self, agent_id: str, workspace_id: str) -> Optional[Dict[str,
Any]] (or whatever the protocol specifies for agent config), use the passed
workspace_id in the queries instead of self.workspace_id, and change
get_workspace_context to return the workspace_id (or None) rather than a dict so
the class conforms to WorkspaceContextProtocol; update any consumers to pass
workspace_id accordingly.
|
@claude You are the FINAL architecture reviewer. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task —— View job Final Architecture ReviewPhase 1: Architecture Review per AGENTS.md
Phase 2: Critical Issues Found
Phase 3: Final Verdict
🚨 BLOCKING PROTOCOL VIOLATIONS (P0)The Issue 1:
|
…otocol Agent-Logs-Url: https://github.com/MervinPraison/PraisonAI/sessions/4f65e6a4-b164-458e-ad64-6141c6d529c0 Co-authored-by: MervinPraison <454862+MervinPraison@users.noreply.github.com>
Completed a full PR review and incorporated the actionable reviewer finding in commit Key results:
Suggested next improvement (optional): add focused unit tests for |
|
@claude You are the FINAL architecture reviewer. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md Protocol-driven: check heavy implementations vs core SDK Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task —— View job Final Architecture Review ✅Phase 1: Review per AGENTS.md
Phase 2: Fix Valid Issues
Phase 3: Final Verdict
✅ ARCHITECTURE REVIEW COMPLETE - APPROVEDIssues Fixed (Commit c25c1cc)1. Protocol Key Mismatch (P0) - RESOLVED
2. Defensive Copying (P1) - RESOLVED
Architecture Compliance Assessment✅ Protocol-driven design: Correctly implements Files Modified
What Was NOT Done (Optional Improvements)
Final Verdict: ✅ READY TO MERGEAll blocking protocol violations have been resolved. The implementation now properly conforms to |
…otocol - Add protocol-expected keys: system_prompt, model, tools - Map agent.instructions to system_prompt for protocol compliance - Add defensive copying with deepcopy for mutable fields - Maintain platform-specific keys for backward compatibility - Fix docstring to document protocol + platform keys Resolves protocol signature mismatches identified in review. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Summary
Implements
PlatformWorkspaceContext— a class that provides workspace-level context and agent configuration by querying the database, implementing theWorkspaceContextProtocolfrompraisonaiagents.auth.protocols.Implementation Details
PlatformWorkspaceContext class
WorkspaceContextProtocolfrompraisonaiagents.auth.protocolsworkspace_idand anAsyncSessionfor database queriesget_workspace_context()— queriesWorkspacemodel, returns dict withid,name,slug,description,settingsget_agent_config(agent_id)— queriesAgentmodel scoped to the workspace, returns dict withid,name,runtime_mode,instructions,config,max_concurrent_tasksNoneif workspace or agent not foundServices exports
PlatformWorkspaceContextfrompraisonai_platform.servicesmoduleFiles Changed
src/praisonai-platform/praisonai_platform/services/workspace_context.py(NEW)src/praisonai-platform/praisonai_platform/services/__init__.py(updated exports)Usage Example
Acceptance Criteria
WorkspaceContextProtocolinterfaceNonefor missing workspace/agentpraisonai_platform.servicesFixes #1374
🤖 Generated with Claude Code
Summary by CodeRabbit