mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 12:16:54 +00:00
feat(tools): add team workspace context + WorkspaceInterceptor + file tools access
- Add WithToolTeamWorkspace/ToolTeamWorkspaceFromCtx context key for team workspace path (accessible but not necessarily default) - Create WorkspaceInterceptor for team-specific write validation (RBAC, quota, blocked extensions, event broadcasting) - File tools (read_file, write_file, list_files, edit) allow access to team workspace via allowedWithTeamWorkspace() helper - read_file/list_files hint team workspace path when file not found - Registry detects empty tool call args and returns actionable hint (DashScope/Qwen large-output truncation workaround)
This commit is contained in:
1 parent
27fb900510
commit
8d7259f637
7 files changed
+227
-8
No files matched your search
@@ -224,8 +224,8 @@ func MemoryConfigFromCtx(ctx context.Context) *config.MemoryConfig {
|
||||
|
||||
const ctxTeamID toolContextKey = "tool_team_id"
|
||||
|
||||
// WithToolTeamID injects the dispatching team's ID into context so workspace
|
||||
// tools (workspace_read, workspace_write, team_tasks, team_message) resolve
|
||||
// WithToolTeamID injects the dispatching team's ID into context so team
|
||||
// tools (team_tasks, team_message) and the WorkspaceInterceptor resolve
|
||||
// the correct team when the agent belongs to multiple teams.
|
||||
func WithToolTeamID(ctx context.Context, teamID string) context.Context {
|
||||
return context.WithValue(ctx, ctxTeamID, teamID)
|
||||
@@ -237,6 +237,22 @@ func ToolTeamIDFromCtx(ctx context.Context) string {
|
||||
return v
|
||||
}
|
||||
|
||||
// --- Team workspace path (accessible but not default) ---
|
||||
|
||||
const ctxTeamWorkspace toolContextKey = "tool_team_workspace"
|
||||
|
||||
// WithToolTeamWorkspace stores the team shared workspace directory path.
|
||||
// File tools allow access to this path even when restrict_to_workspace is true.
|
||||
func WithToolTeamWorkspace(ctx context.Context, dir string) context.Context {
|
||||
return context.WithValue(ctx, ctxTeamWorkspace, dir)
|
||||
}
|
||||
|
||||
// ToolTeamWorkspaceFromCtx returns the team shared workspace directory path.
|
||||
func ToolTeamWorkspaceFromCtx(ctx context.Context) string {
|
||||
v, _ := ctx.Value(ctxTeamWorkspace).(string)
|
||||
return v
|
||||
}
|
||||
|
||||
// --- Team task ID propagation (delegation origin → workspace tools) ---
|
||||
|
||||
const ctxTeamTaskID toolContextKey = "tool_team_task_id"
|
||||
|
||||
@@ -160,7 +160,8 @@ func (t *EditTool) Execute(ctx context.Context, args map[string]any) *Result {
|
||||
if workspace == "" {
|
||||
workspace = t.workspace
|
||||
}
|
||||
resolved, err := resolvePath(path, workspace, effectiveRestrict(ctx, t.restrict))
|
||||
allowed := allowedWithTeamWorkspace(ctx, nil)
|
||||
resolved, err := resolvePathWithAllowed(path, workspace, effectiveRestrict(ctx, t.restrict), allowed)
|
||||
if err != nil {
|
||||
return ErrorResult(err.Error())
|
||||
}
|
||||
|
||||
@@ -145,7 +145,8 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
if workspace == "" {
|
||||
workspace = t.workspace
|
||||
}
|
||||
resolved, err := resolvePathWithAllowed(path, workspace, effectiveRestrict(ctx, t.restrict), t.allowedPrefixes)
|
||||
allowed := allowedWithTeamWorkspace(ctx, t.allowedPrefixes)
|
||||
resolved, err := resolvePathWithAllowed(path, workspace, effectiveRestrict(ctx, t.restrict), allowed)
|
||||
if err != nil {
|
||||
return ErrorResult(err.Error())
|
||||
}
|
||||
@@ -155,7 +156,13 @@ func (t *ReadFileTool) Execute(ctx context.Context, args map[string]any) *Result
|
||||
|
||||
data, err := os.ReadFile(resolved)
|
||||
if err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
|
||||
msg := fmt.Sprintf("failed to read file: %v", err)
|
||||
if os.IsNotExist(err) {
|
||||
if teamWs := ToolTeamWorkspaceFromCtx(ctx); teamWs != "" && !strings.HasPrefix(resolved, teamWs) {
|
||||
msg += fmt.Sprintf("\nHint: file may be in the team workspace. Try: read_file(path=\"%s/%s\")", teamWs, path)
|
||||
}
|
||||
}
|
||||
return ErrorResult(msg)
|
||||
}
|
||||
|
||||
return SilentResult(string(data))
|
||||
@@ -183,6 +190,19 @@ func (t *ReadFileTool) getFsBridge(ctx context.Context, sandboxKey string) (*san
|
||||
return sandbox.NewFsBridge(sb.ID(), "/workspace"), nil
|
||||
}
|
||||
|
||||
// allowedWithTeamWorkspace returns the allowed prefixes with team workspace appended
|
||||
// if present in context. Thread-safe: creates a new slice per request.
|
||||
func allowedWithTeamWorkspace(ctx context.Context, base []string) []string {
|
||||
teamWs := ToolTeamWorkspaceFromCtx(ctx)
|
||||
if teamWs == "" {
|
||||
return base
|
||||
}
|
||||
out := make([]string, len(base)+1)
|
||||
copy(out, base)
|
||||
out[len(base)] = teamWs
|
||||
return out
|
||||
}
|
||||
|
||||
// resolvePathWithAllowed is like resolvePath but also allows paths under extra prefixes.
|
||||
func resolvePathWithAllowed(path, workspace string, restrict bool, allowedPrefixes []string) (string, error) {
|
||||
resolved, err := resolvePath(path, workspace, restrict)
|
||||
|
||||
@@ -88,7 +88,8 @@ func (t *ListFilesTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
if workspace == "" {
|
||||
workspace = t.workspace
|
||||
}
|
||||
resolved, err := resolvePath(path, workspace, effectiveRestrict(ctx, t.restrict))
|
||||
allowed := allowedWithTeamWorkspace(ctx, nil)
|
||||
resolved, err := resolvePathWithAllowed(path, workspace, effectiveRestrict(ctx, t.restrict), allowed)
|
||||
if err != nil {
|
||||
return ErrorResult(err.Error())
|
||||
}
|
||||
@@ -99,7 +100,11 @@ func (t *ListFilesTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
entries, err := os.ReadDir(resolved)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
return SilentResult(fmt.Sprintf("Directory does not exist: %s", path))
|
||||
msg := fmt.Sprintf("Directory does not exist: %s", path)
|
||||
if teamWs := ToolTeamWorkspaceFromCtx(ctx); teamWs != "" && !strings.HasPrefix(resolved, teamWs) {
|
||||
msg += fmt.Sprintf("\nHint: try the team workspace path: list_files(path=\"%s/%s\")", teamWs, path)
|
||||
}
|
||||
return SilentResult(msg)
|
||||
}
|
||||
return ErrorResult(fmt.Sprintf("failed to list directory: %v", err))
|
||||
}
|
||||
|
||||
@@ -20,6 +20,7 @@ type WriteFileTool struct {
|
||||
contextFileIntc *ContextFileInterceptor // nil = no virtual FS routing
|
||||
memIntc *MemoryInterceptor // nil = no memory routing
|
||||
groupWriterCache *store.GroupWriterCache // nil = no group write restriction
|
||||
workspaceIntc *WorkspaceInterceptor // nil = no team workspace validation
|
||||
}
|
||||
|
||||
// DenyPaths adds path prefixes that write_file must reject.
|
||||
@@ -42,6 +43,11 @@ func (t *WriteFileTool) SetGroupWriterCache(c *store.GroupWriterCache) {
|
||||
t.groupWriterCache = c
|
||||
}
|
||||
|
||||
// SetWorkspaceInterceptor enables team workspace validation and event broadcasting.
|
||||
func (t *WriteFileTool) SetWorkspaceInterceptor(intc *WorkspaceInterceptor) {
|
||||
t.workspaceIntc = intc
|
||||
}
|
||||
|
||||
func NewWriteFileTool(workspace string, restrict bool) *WriteFileTool {
|
||||
return &WriteFileTool{workspace: workspace, restrict: restrict}
|
||||
}
|
||||
@@ -128,7 +134,8 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
if workspace == "" {
|
||||
workspace = t.workspace
|
||||
}
|
||||
resolved, err := resolvePath(path, workspace, effectiveRestrict(ctx, t.restrict))
|
||||
allowed := allowedWithTeamWorkspace(ctx, nil)
|
||||
resolved, err := resolvePathWithAllowed(path, workspace, effectiveRestrict(ctx, t.restrict), allowed)
|
||||
if err != nil {
|
||||
return ErrorResult(err.Error())
|
||||
}
|
||||
@@ -136,6 +143,21 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
return ErrorResult(err.Error())
|
||||
}
|
||||
|
||||
// Team workspace validation + delete-on-empty.
|
||||
if t.workspaceIntc != nil {
|
||||
isDelete, intcErr := t.workspaceIntc.HandleWrite(ctx, resolved, content)
|
||||
if intcErr != nil {
|
||||
return ErrorResult(intcErr.Error())
|
||||
}
|
||||
if isDelete {
|
||||
if err := os.Remove(resolved); err != nil && !os.IsNotExist(err) {
|
||||
return ErrorResult(fmt.Sprintf("failed to delete file: %v", err))
|
||||
}
|
||||
t.workspaceIntc.AfterWrite(ctx, resolved, "delete")
|
||||
return SilentResult(fmt.Sprintf("File deleted: %s", path))
|
||||
}
|
||||
}
|
||||
|
||||
if err := os.MkdirAll(filepath.Dir(resolved), 0755); err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to create directory: %v", err))
|
||||
}
|
||||
@@ -144,6 +166,10 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
|
||||
}
|
||||
|
||||
if t.workspaceIntc != nil {
|
||||
t.workspaceIntc.AfterWrite(ctx, resolved, "write")
|
||||
}
|
||||
|
||||
result := SilentResult(fmt.Sprintf("File written: %s (%d bytes)", path, len(content)))
|
||||
result.Deliverable = content
|
||||
if deliver {
|
||||
|
||||
@@ -2,8 +2,10 @@ package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"maps"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
@@ -136,6 +138,21 @@ func (r *Registry) ExecuteWithContext(ctx context.Context, name string, args map
|
||||
}
|
||||
}
|
||||
|
||||
// Detect empty tool call arguments — typically caused by providers truncating
|
||||
// or dropping arguments when output is too large (e.g. DashScope with long content).
|
||||
// Give the model an actionable hint instead of a confusing "X is required" error.
|
||||
if len(args) == 0 {
|
||||
if params := tool.Parameters(); params != nil {
|
||||
if req, ok := params["required"].([]string); ok && len(req) > 0 {
|
||||
return ErrorResult(fmt.Sprintf(
|
||||
"Tool call had empty arguments (required: %s). "+
|
||||
"This usually means your previous response was too long for the API to include tool parameters. "+
|
||||
"Try again with shorter content — split into smaller parts if needed.",
|
||||
strings.Join(req, ", ")))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
start := time.Now()
|
||||
result := tool.Execute(ctx, args)
|
||||
duration := time.Since(start)
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
|
||||
)
|
||||
|
||||
// WorkspaceInterceptor validates writes and broadcasts events for team workspace files.
|
||||
// When no team context is active (ToolTeamIDFromCtx returns ""), all methods are no-ops.
|
||||
type WorkspaceInterceptor struct {
|
||||
teamMgr *TeamToolManager
|
||||
}
|
||||
|
||||
func NewWorkspaceInterceptor(mgr *TeamToolManager) *WorkspaceInterceptor {
|
||||
return &WorkspaceInterceptor{teamMgr: mgr}
|
||||
}
|
||||
|
||||
// HandleWrite validates a file write in team workspace context.
|
||||
// Returns (true, nil) if the write should be treated as a delete (empty content).
|
||||
// Returns (false, nil) to proceed with normal write.
|
||||
// Returns (_, error) to block the write.
|
||||
func (w *WorkspaceInterceptor) HandleWrite(ctx context.Context, path string, content string) (isDelete bool, err error) {
|
||||
if w == nil {
|
||||
return false, nil
|
||||
}
|
||||
teamIDStr := ToolTeamIDFromCtx(ctx)
|
||||
if teamIDStr == "" {
|
||||
return false, nil // Not in team context
|
||||
}
|
||||
|
||||
// Only apply team validation when path is inside the team workspace.
|
||||
teamWs := ToolTeamWorkspaceFromCtx(ctx)
|
||||
if teamWs == "" || !strings.HasPrefix(filepath.Clean(path), filepath.Clean(teamWs)) {
|
||||
return false, nil // Write is to agent's own workspace, not team workspace
|
||||
}
|
||||
|
||||
// Resolve team and role for RBAC. Fail-open: if resolution fails (DB issue,
|
||||
// corrupt cache), allow the write but log a warning for observability.
|
||||
team, agentID, err := w.teamMgr.resolveTeam(ctx)
|
||||
if err != nil {
|
||||
slog.Warn("workspace: team resolution failed, skipping validation", "team", teamIDStr, "error", err)
|
||||
return false, nil
|
||||
}
|
||||
role, err := w.teamMgr.resolveTeamRole(ctx, team, agentID)
|
||||
if err != nil {
|
||||
slog.Warn("workspace: role resolution failed, skipping validation", "team", teamIDStr, "error", err)
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// Empty content = delete.
|
||||
if content == "" {
|
||||
if role == store.TeamRoleReviewer {
|
||||
return false, fmt.Errorf("reviewers cannot delete workspace files")
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// RBAC: reviewer cannot write.
|
||||
if role == store.TeamRoleReviewer {
|
||||
return false, fmt.Errorf("reviewers cannot write to the workspace")
|
||||
}
|
||||
|
||||
// Blocked extensions.
|
||||
ext := strings.ToLower(filepath.Ext(path))
|
||||
if blockedExtensions[ext] {
|
||||
return false, fmt.Errorf("executable file type %q is not allowed", ext)
|
||||
}
|
||||
|
||||
// File size limit (10MB).
|
||||
if len(content) > maxFileSizeBytes {
|
||||
return false, fmt.Errorf("file exceeds max size (10MB)")
|
||||
}
|
||||
|
||||
// Quota: count files in team workspace scope.
|
||||
wsDir := teamWs
|
||||
if wsDir != "" {
|
||||
entries, err := os.ReadDir(wsDir)
|
||||
if err != nil {
|
||||
slog.Warn("workspace: quota check ReadDir failed", "dir", wsDir, "error", err)
|
||||
}
|
||||
fileCount := 0
|
||||
for _, e := range entries {
|
||||
if !e.IsDir() {
|
||||
fileCount++
|
||||
}
|
||||
}
|
||||
// Only check when creating new file (not updating existing).
|
||||
if _, statErr := os.Stat(path); os.IsNotExist(statErr) {
|
||||
if fileCount >= maxFilesPerScope {
|
||||
return false, fmt.Errorf("workspace file limit reached (%d/%d)", fileCount, maxFilesPerScope)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// AfterWrite broadcasts a workspace file change event.
|
||||
func (w *WorkspaceInterceptor) AfterWrite(ctx context.Context, path string, action string) {
|
||||
if w == nil {
|
||||
return
|
||||
}
|
||||
teamIDStr := ToolTeamIDFromCtx(ctx)
|
||||
if teamIDStr == "" {
|
||||
return
|
||||
}
|
||||
// Only broadcast for writes inside team workspace.
|
||||
teamWs := ToolTeamWorkspaceFromCtx(ctx)
|
||||
if teamWs == "" || !strings.HasPrefix(filepath.Clean(path), filepath.Clean(teamWs)) {
|
||||
return
|
||||
}
|
||||
|
||||
fileName := filepath.Base(path)
|
||||
chatID := ToolChatIDFromCtx(ctx)
|
||||
if chatID == "" {
|
||||
chatID = store.UserIDFromContext(ctx)
|
||||
}
|
||||
|
||||
w.teamMgr.broadcastTeamEvent(protocol.EventWorkspaceFileChanged, map[string]string{
|
||||
"team_id": teamIDStr,
|
||||
"channel": "",
|
||||
"chat_id": chatID,
|
||||
"file_name": fileName,
|
||||
"action": action,
|
||||
})
|
||||
slog.Debug("workspace: file changed", "team", teamIDStr, "file", fileName, "action", action)
|
||||
}
|
||||
Reference in new issue
Block a user