fix(core): apply global workspace to registered agents#1228
Merged
Conversation
🦋 Changeset detectedLatest commit: 6b2552f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment has been minimized.
This comment has been minimized.
Contributor
📝 WalkthroughWalkthroughThis pull request implements automatic propagation of global workspace defaults to agents created before registration. When a Changes
Sequence DiagramsequenceDiagram
participant User
participant VoltAgent
participant Agent
participant Workspace
participant Registry
User->>Agent: new Agent({ tools: [...] })
User->>VoltAgent: new VoltAgent({ agents, workspace })
VoltAgent->>Workspace: Initialize workspace
VoltAgent->>Registry: Fetch workspace from registry
VoltAgent->>Agent: applyDefaultWorkspaceToAgent()
Agent->>Agent: __setDefaultWorkspace(workspace)
Agent->>Agent: Set workspace, update maxSteps
Agent->>Agent: Recompute onPrepareMessages hook
Agent->>Agent: Register workspace toolkits
VoltAgent->>VoltAgent: Complete initialization
User->>Agent: generateText(prompt)
Agent->>Agent: Access workspace tools via onPrepareMessages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/src/agent/agent.ts (1)
981-1008: Prefer private backing state over widening public mutability.
hooksandmaxStepsappear mutable only so__setDefaultWorkspacecan update internal defaults. Keeping public getters with private backing fields would preserve the previous read-only API shape and avoid consumers mutating values without updating constructor-time flags likemaxStepsConfigured/configuredHooks. As per coding guidelines, Maintain type safety in TypeScript-first codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/agent/agent.ts` around lines 981 - 1008, The fields hooks and maxSteps should be made private-backed (e.g., private _hooks and private _maxSteps) and exposed via public getters so external code cannot mutate them directly; update the constructor to initialize the private backing fields and change any access sites to use the getters, then modify __setDefaultWorkspace to assign the backing fields (setting configuredHooks and maxStepsConfigured as it already does) instead of mutating the public properties; ensure references to configuredHooks and maxStepsConfigured remain correct and remove or rename the original public mutable fields (hooks, maxSteps) so the public API remains read-only while internal defaults can still be updated via the private backing fields.packages/core/src/voltagent.spec.ts (1)
62-87: Add coverage for the explicit workspace opt-out path.This regression covers inherited global workspace, but the PR also needs to preserve agents constructed with
workspace: false. A small companion assertion would protect that behavior from regressing.Suggested test addition
it("applies workspace to preconstructed registered agents without explicit workspace", async () => { const workspace = new Workspace({ id: "global-workspace" }); const agent = new Agent({ name: "assistant", instructions: "Be helpful.", model: "openai/gpt-4o-mini", }); + const optOutAgent = new Agent({ + name: "no-workspace", + instructions: "Be helpful.", + model: "openai/gpt-4o-mini", + workspace: false, + }); expect(agent.getWorkspace()).toBeUndefined(); expect(agent.getTools()).toHaveLength(0); const voltAgent = new VoltAgent({ - agents: { assistant: agent }, + agents: { assistant: agent, noWorkspace: optOutAgent }, workspace, checkDependencies: false, }); expect(agent.getWorkspace()).toBe(workspace); expect(agent.maxSteps).toBe(100); expect(agent.getTools().map((tool) => tool.name)).toEqual( expect.arrayContaining(["ls", "read_file", "execute_command", "workspace_search"]), ); + expect(optOutAgent.getWorkspace()).toBeUndefined(); + expect(optOutAgent.getTools().map((tool) => tool.name)).not.toEqual( + expect.arrayContaining(["ls", "read_file", "execute_command", "workspace_search"]), + ); await voltAgent.ready; await voltAgent.shutdown(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/voltagent.spec.ts` around lines 62 - 87, Add a test case that verifies an agent constructed with an explicit workspace opt-out keeps workspace unset after being passed into VoltAgent: create a second Agent with workspace: false (e.g., new Agent({ name: "outsider", ..., workspace: false })), include it in the VoltAgent constructor alongside the existing agent and global Workspace, then assert that outsider.getWorkspace() remains undefined while the other preconstructed agent still inherits the global Workspace; reference the existing VoltAgent, Agent, and Workspace usage in the spec to locate where to add this assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/agent/agent.ts`:
- Around line 981-1008: The fields hooks and maxSteps should be made
private-backed (e.g., private _hooks and private _maxSteps) and exposed via
public getters so external code cannot mutate them directly; update the
constructor to initialize the private backing fields and change any access sites
to use the getters, then modify __setDefaultWorkspace to assign the backing
fields (setting configuredHooks and maxStepsConfigured as it already does)
instead of mutating the public properties; ensure references to configuredHooks
and maxStepsConfigured remain correct and remove or rename the original public
mutable fields (hooks, maxSteps) so the public API remains read-only while
internal defaults can still be updated via the private backing fields.
In `@packages/core/src/voltagent.spec.ts`:
- Around line 62-87: Add a test case that verifies an agent constructed with an
explicit workspace opt-out keeps workspace unset after being passed into
VoltAgent: create a second Agent with workspace: false (e.g., new Agent({ name:
"outsider", ..., workspace: false })), include it in the VoltAgent constructor
alongside the existing agent and global Workspace, then assert that
outsider.getWorkspace() remains undefined while the other preconstructed agent
still inherits the global Workspace; reference the existing VoltAgent, Agent,
and Workspace usage in the spec to locate where to add this assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4680422-f583-4644-86b5-df40bced7ae8
📒 Files selected for processing (4)
.changeset/global-workspace-default.mdpackages/core/src/agent/agent.tspackages/core/src/voltagent.spec.tspackages/core/src/voltagent.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Deploying voltagent with
|
| Latest commit: |
6b2552f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f26fe90c.voltagent.pages.dev |
| Branch Preview URL: | https://fix-global-workspace-default.voltagent.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
Agents constructed before
new VoltAgent({ workspace })do not inherit the global workspace. This means workspace toolkits are missing unless the workspace is passed directly to theAgentconstructor.What is the new behavior?
VoltAgentnow applies the configured global workspace to registered agents that did not explicitly configure a workspace. This adds the workspace toolkits, preserves explicitworkspace: false, and keeps workspace defaults such asmaxSteps.fixes #1227
Notes for reviewers
Reproduced in
examples/with-workspace: before the fix the agent hadworkspace missingand onlycurrentDate; after the fix it has the workspace id and the expected workspace tools.Validation run:
pnpm --filter @voltagent/core test:single -- src/voltagent.spec.ts src/workspace/index.spec.tspnpm --filter @voltagent/core typecheckpnpm --filter @voltagent/core buildpnpm --dir examples/with-workspace buildpnpm biome check .changeset/global-workspace-default.md packages/core/src/agent/agent.ts packages/core/src/voltagent.ts packages/core/src/voltagent.spec.tsSummary by cubic
Fixes agents created before registration not inheriting the global workspace. Registered agents without an explicit workspace now get workspace defaults and toolkits. Fixes #1227 in
@voltagent/core.workspace: false.maxSteps(100) and apply the workspace skills prompt hook when applicable.Written for commit 6b2552f. Summary will update on new commits.
Summary by CodeRabbit