mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-12 10:13:37 +00:00
* fix(teams): scope team tasks to the delegation origin, not the delivery channel
A delegated run is delivered on the internal "delegate" channel while its real
origin is preserved separately — buildAgentLinkRunRequest is explicit about it:
// preserves the origin's authorization-bearing identity while keeping
// delegation on its internal delivery channel.
Channel: "delegate",
WorkspaceChannel: req.Channel,
WorkspaceChatID: req.ChatID,
The team tools did not consult that origin, so a task created by a lead reached
through delegate was stamped with the delivery channel. Nothing is registered
for it, so every notification about that task — completion, failure, blocker
escalation, ask_user — was dropped:
unknown channel for outbound message channel=delegate
The delegatee's first answer still arrived, because it travels back as the
delegation result rather than through a channel; everything the lead said after
the delegation closed was lost. One session produced 16 such drops, including a
blocker escalation the user needed to see.
Add OriginChannelFromCtx / OriginChatIDFromCtx next to the existing workspace
scope propagation helpers, and use them where team scoping and notification
routing are decided: task records, list/search scoping, dispatch fallbacks,
event payloads, escalation tasks, ask_user and leader notifications. The
resolution is the identity when no delegation origin is present, so
non-delegated flows are unchanged.
Deliberately not touched: the channel used in authorization decisions
(checkTeamAccess, requireLead, approve/reject lead bypass). Those ask "how did
this call arrive", not "where should the answer go", and widening them is a
separate question.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(teams): cover delegated-lead completion routing and origin isolation
Triage on #1529 named two gates this PR had not met: regression coverage for
delegated lead task completion, and a guard that unrelated origins cannot
receive the notification. The existing tests only covered what create persists.
TestDelegatedLeadCompletionNotifiesOrigin completes a task raised in a delegated
context and asserts the completion event is addressed to the caller's origin.
Reverting team_event_helpers.go to the delivery channel fails it with
"addressed to delegate/system".
TestCompletionNotificationStaysWithinItsOwnOrigin puts two tasks from different
origins on one board, completes one, and asserts no completion notification
carries the other origin.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(teams): grant a delegated lead read access to its own team
Fixes #1535. Second attempt: the first one set RunRequest.TeamWorkspace on the
delegated run, which loop_context.go rejects outright and deliberately so — that
field also becomes ToolWorkspace, which would displace the exchange outputs
directory. Every delegation then died at setup with "invalid delegation artifact
workspace". This does not go near that field.
The separation the lead needs already exists in the code. A lead addressed
directly resolves its team at loop_context.go:285-321 and gets
ctx = tools.WithToolTeamWorkspace(ctx, wsDir)
ctx = tools.WithToolTeamRoot(ctx, teamRoot)
with no WithToolWorkspace call — a read allowance, not an override. Only the
!isArtifactDelegation gate keeps a delegated lead out of it. So the grant is
made inside the artifact branch instead, from the same inputs, and the guard on
req.TeamWorkspace stays exactly as strict as it was.
Both paths are needed. The workspace alone covers only the lead's own chat leaf;
the deliverable in the report sits at teams/<id>/system/review-....md, written by
a member under a different chat scope. buildAllowedPrefixes adds the team root
for reads and not for writes (filesystem.go:366-370), which is precisely the
asymmetry this case wants: the lead reads the team's output, per-chat write
isolation is untouched.
Team ID is deliberately not set. It switches on the workspace interceptor's
write validation, file-change broadcast and task attachment (workspace_
interceptor.go:36,114,203) — none of which a delegated run should trigger, and
none of which reading needs.
Ambiguity is not resolved silently, as triage asked: an agent leading more than
one active team gets nothing. store.GetTeamForAgent would answer in one call but
it is ORDER BY (lead_agent_id = $1) DESC LIMIT 1 and would quietly pick a team.
Hermeticity is unchanged: send_file and message still refuse inside an artifact
run, publication still happens through delegation outputs on completion.
Tests: TestInjectContext_DelegatedLeadReadsTeamWithoutDisplacingOutputs drives
injectContext through the guard that broke the first attempt and pins both
halves — the run is not refused, and ToolWorkspace stays the outputs directory.
Resolution rules, the shared/isolated path shapes, read-not-write on the team
root, and send_file staying blocked are covered alongside.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(teams): pin that the delegated lead resolver does not re-scope by tenant
l.dataDir arrives already tenant-scoped from resolver.go (config.TenantDataDir),
which is why resolveDelegatedLeadTeamRead must not apply TenantLayer itself.
That was carried only by a comment, here and in the sibling branch of
injectContext, and nothing failed if someone added the layer back.
The failure it guards against is quiet: the path stays plausible, it just gains
a second tenant segment — teams/<id> under tenants/<slug>/tenants/<slug> — so
the lead is handed a directory its own tasks never write to, and reads come back
empty rather than denied.
Verified the assertion has teeth by reintroducing the layer: the subtest fails
with "tenant segment appears 2 times". Prompted by #1546 in this same area,
where a contract that lived outside the unit under test went unpinned and the
feature shipped green and did nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
344 lines
13 KiB
Go
344 lines
13 KiB
Go
package tools
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"log/slog"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
|
|
"github.com/google/uuid"
|
|
|
|
"github.com/nextlevelbuilder/goclaw/internal/store"
|
|
"github.com/nextlevelbuilder/goclaw/internal/tracing"
|
|
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
|
|
)
|
|
|
|
func (t *TeamTasksTool) executeCreate(ctx context.Context, args map[string]any) *Result {
|
|
team, agentID, err := t.manager.ResolveTeam(ctx)
|
|
if err != nil {
|
|
return ErrorResult(err.Error())
|
|
}
|
|
|
|
// Determine if caller is a lead or a member.
|
|
isLead := agentID == team.LeadAgentID
|
|
channel := OriginChannelFromCtx(ctx)
|
|
if channel == ChannelTeammate || channel == ChannelSystem {
|
|
isLead = true // system/teammate channels act on behalf of the lead
|
|
}
|
|
|
|
taskType, _ := args["task_type"].(string)
|
|
if taskType == "" {
|
|
taskType = "general"
|
|
}
|
|
|
|
if !isLead {
|
|
// Members may only create "request" tasks when the feature is enabled.
|
|
memberCfg := ParseMemberRequestConfig(team.Settings)
|
|
if !memberCfg.Enabled {
|
|
return ErrorResult("Members cannot create tasks. Use team_tasks(action=\"comment\") to communicate.")
|
|
}
|
|
if taskType != "request" {
|
|
return ErrorResult("Members can only create task_type=\"request\". Use team_tasks(action=\"comment\") to communicate.")
|
|
}
|
|
} else if err := t.manager.RequireLead(ctx, team, agentID); err != nil {
|
|
return ErrorResult(err.Error())
|
|
}
|
|
|
|
// Gate: must list tasks before creating to prevent duplicates in concurrent group chat.
|
|
if ptd := PendingTeamDispatchFromCtx(ctx); ptd != nil && !ptd.HasListed() {
|
|
return ErrorResult("You must check existing tasks first. Call team_tasks(action=\"search\", query=\"<keywords>\") to check for similar tasks before creating — this saves tokens vs listing all. Alternatively use action=\"list\" to see the full board.")
|
|
}
|
|
|
|
subject, _ := args["subject"].(string)
|
|
if subject == "" {
|
|
return ErrorResult("subject is required for create action")
|
|
}
|
|
|
|
description, _ := args["description"].(string)
|
|
priority := 0
|
|
if p, ok := args["priority"].(float64); ok {
|
|
priority = int(p)
|
|
}
|
|
|
|
var blockedBy []uuid.UUID
|
|
if raw, ok := args["blocked_by"].([]any); ok {
|
|
for _, v := range raw {
|
|
if s, ok := v.(string); ok {
|
|
id, err := uuid.Parse(s)
|
|
if err != nil {
|
|
return ErrorResult(fmt.Sprintf("blocked_by contains invalid task ID %q — must be a real task UUID from a previous create call. Create dependency tasks first, then use their IDs.", s))
|
|
}
|
|
blockedBy = append(blockedBy, id)
|
|
}
|
|
}
|
|
}
|
|
|
|
// Validate that all blocked_by tasks belong to the same team and are not terminal.
|
|
for _, depID := range blockedBy {
|
|
depTask, err := t.manager.Store().GetTask(ctx, depID)
|
|
if err != nil {
|
|
return ErrorResult(fmt.Sprintf("blocked_by task %s not found: %v", depID, err))
|
|
}
|
|
if depTask.TeamID != team.ID {
|
|
return ErrorResult(fmt.Sprintf("blocked_by task %s belongs to a different team", depID))
|
|
}
|
|
switch depTask.Status {
|
|
case store.TeamTaskStatusCompleted, store.TeamTaskStatusCancelled, store.TeamTaskStatusFailed:
|
|
return ErrorResult(fmt.Sprintf(
|
|
"blocked_by task %s (%s) is already %s. "+
|
|
"Do not block on finished tasks — create this task without blocked_by, "+
|
|
"or pass the completed task's result in the description instead.",
|
|
depID, depTask.Subject, depTask.Status))
|
|
}
|
|
}
|
|
|
|
// Resolve assignee (agent key → UUID). Required — every task must be assigned.
|
|
assigneeKey, _ := args["assignee"].(string)
|
|
if assigneeKey == "" {
|
|
return ErrorResult("assignee is required — specify which team member should handle this task")
|
|
}
|
|
assigneeID, err := t.manager.ResolveAgentByKey(ctx, assigneeKey)
|
|
if err != nil {
|
|
return ErrorResult(fmt.Sprintf("assignee %q not found: %v", assigneeKey, err))
|
|
}
|
|
// Verify assignee is a member of this team.
|
|
members, err := t.manager.CachedListMembers(ctx, team.ID, agentID)
|
|
if err != nil {
|
|
return ErrorResult("failed to verify team membership: " + err.Error())
|
|
}
|
|
isMember := false
|
|
for _, m := range members {
|
|
if m.AgentID == assigneeID {
|
|
isMember = true
|
|
break
|
|
}
|
|
}
|
|
if !isMember {
|
|
return ErrorResult(fmt.Sprintf("agent %q is not a member of this team", assigneeKey))
|
|
}
|
|
// Prevent lead from self-assigning — causes dual-session execution + loop.
|
|
// buildCreateHint already hides the lead from the member list hint,
|
|
// but weaker models may still attempt self-assignment.
|
|
if assigneeID == team.LeadAgentID {
|
|
return ErrorResult("team lead cannot assign tasks to itself — delegate to a team member instead. You are the team lead; handle this work directly or assign to one of your members.")
|
|
}
|
|
|
|
requireApproval, _ := args["require_approval"].(bool)
|
|
status := store.TeamTaskStatusPending
|
|
if requireApproval {
|
|
status = store.TeamTaskStatusInReview
|
|
} else if len(blockedBy) > 0 {
|
|
status = store.TeamTaskStatusBlocked
|
|
}
|
|
// Assigned tasks without blockers stay pending — dispatched after the turn
|
|
// ends via post-turn processing (avoids race with blocked_by setup).
|
|
|
|
// Member requests without auto_dispatch stay pending for leader review.
|
|
memberCfgForDispatch := ParseMemberRequestConfig(team.Settings)
|
|
skipAutoDispatch := !isLead && taskType == "request" && !memberCfgForDispatch.AutoDispatch
|
|
|
|
chatID := OriginChatIDFromCtx(ctx)
|
|
|
|
// Compute team workspace via layered pipeline: tenant → team → user/chat.
|
|
shared := IsSharedWorkspace(team.Settings)
|
|
taskMeta := make(map[string]any)
|
|
teamWsDir := ResolveWorkspace(t.manager.DataDir(),
|
|
TenantLayer(store.TenantIDFromContext(ctx), store.TenantSlugFromContext(ctx)),
|
|
TeamLayer(team.ID),
|
|
UserChatLayer(chatID, shared),
|
|
)
|
|
taskMeta[TaskMetaTeamWorkspace] = teamWsDir
|
|
// Auto-collect media files from current run to team workspace.
|
|
// When leader received files from user and creates a task, copy those
|
|
// files to the team workspace so members can access them via read_file.
|
|
// Also rewrite any media paths in the description to point to the workspace copy,
|
|
// since members can't access the original .media/ paths outside their workspace.
|
|
if mediaPaths := RunMediaPathsFromCtx(ctx); len(mediaPaths) > 0 {
|
|
if wsDir, _ := taskMeta[TaskMetaTeamWorkspace].(string); wsDir != "" {
|
|
nameMap := RunMediaNamesFromCtx(ctx)
|
|
if copiedPaths := copyMediaToWorkspace(mediaPaths, wsDir, nameMap); len(copiedPaths) > 0 {
|
|
// Store as []any so type assertion works both before and after JSON round-trip.
|
|
files := make([]any, len(copiedPaths))
|
|
for i, p := range copiedPaths {
|
|
files[i] = p
|
|
}
|
|
taskMeta["attached_files"] = files
|
|
|
|
// Rewrite media paths in description so members see workspace paths.
|
|
for i, src := range mediaPaths {
|
|
if i < len(copiedPaths) {
|
|
description = strings.ReplaceAll(description, src, copiedPaths[i])
|
|
}
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
// Preserve original blocked_by list for blocker-result forwarding when task unblocks.
|
|
if len(blockedBy) > 0 {
|
|
ids := make([]string, len(blockedBy))
|
|
for i, id := range blockedBy {
|
|
ids[i] = id.String()
|
|
}
|
|
taskMeta["original_blocked_by"] = ids
|
|
}
|
|
// Store peer kind so dispatches preserve the correct session scope (group vs direct).
|
|
if pk := ToolPeerKindFromCtx(ctx); pk != "" {
|
|
taskMeta[TaskMetaPeerKind] = pk
|
|
}
|
|
// Store local key so forum-topic routing works on deferred/unblocked dispatches.
|
|
if lk := ToolLocalKeyFromCtx(ctx); lk != "" {
|
|
taskMeta[TaskMetaLocalKey] = lk
|
|
}
|
|
// Store origin session key so deferred dispatches route announces correctly.
|
|
// WS sessions use non-standard key format that BuildScopedSessionKey() cannot reproduce.
|
|
if sk := ToolSessionKeyFromCtx(ctx); sk != "" {
|
|
taskMeta[TaskMetaOriginSession] = sk
|
|
}
|
|
// Store leader's trace context so unblocked dispatch links back to the leader's trace.
|
|
if traceID := tracing.TraceIDFromContext(ctx); traceID != uuid.Nil {
|
|
taskMeta[TaskMetaOriginTrace] = traceID.String()
|
|
}
|
|
if rootSpanID := tracing.ParentSpanIDFromContext(ctx); rootSpanID != uuid.Nil {
|
|
taskMeta[TaskMetaOriginRootSpan] = rootSpanID.String()
|
|
}
|
|
// Persist the real acting sender so deferred/dashboard dispatches can
|
|
// restore permission attribution when the teammate runs (#915 Flow F).
|
|
if sender := store.SenderIDFromContext(ctx); sender != "" {
|
|
taskMeta["origin_sender_id"] = sender
|
|
}
|
|
// Persist caller role for RBAC-aware bypass at dispatch time.
|
|
if role := store.RoleFromContext(ctx); role != "" {
|
|
taskMeta["origin_role"] = role
|
|
}
|
|
|
|
task := &store.TeamTaskData{
|
|
TeamID: team.ID,
|
|
Subject: subject,
|
|
Description: description,
|
|
Status: status,
|
|
BlockedBy: blockedBy,
|
|
Priority: priority,
|
|
// SCOPE-intentional (#915 audit 2026-04-16): team task visibility is
|
|
// per-chat, not per-user. team_tasks_read.go filters end-user lists by
|
|
// this same UserID. Migrating to ActorIDFromContext would hide group
|
|
// members' shared work from each other.
|
|
UserID: store.UserIDFromContext(ctx),
|
|
Channel: OriginChannelFromCtx(ctx),
|
|
TaskType: taskType,
|
|
CreatedByAgentID: &agentID,
|
|
ChatID: chatID,
|
|
Metadata: taskMeta,
|
|
}
|
|
task.OwnerAgentID = &assigneeID
|
|
|
|
// Auto-link member request to the member's current task as parent.
|
|
if !isLead && taskType == "request" {
|
|
if parentIDStr := TeamTaskIDFromCtx(ctx); parentIDStr != "" {
|
|
if parentUUID, err := uuid.Parse(parentIDStr); err == nil {
|
|
task.ParentID = &parentUUID
|
|
}
|
|
}
|
|
}
|
|
|
|
if err := t.manager.Store().CreateTask(ctx, task); err != nil {
|
|
return ErrorResult("failed to create task: " + err.Error())
|
|
}
|
|
|
|
// Auto-copy files referenced in subject+description from leader's personal workspace
|
|
// to team workspace so members can access them.
|
|
if isLead {
|
|
personalWs := ToolWorkspaceFromCtx(ctx)
|
|
searchText := subject + "\n" + description
|
|
if n := autoShareFiles(searchText, personalWs, teamWsDir); n > 0 {
|
|
slog.Info("team_tasks.create: auto-shared files", "count", n, "task_id", task.ID)
|
|
}
|
|
}
|
|
|
|
// Persist media files copied during task creation as DB attachments,
|
|
// so the UI and queries see them in team_task_attachments table.
|
|
// (Members get auto-attached via WorkspaceInterceptor, but leaders
|
|
// don't run inside a task context — handle explicitly here.)
|
|
if files, ok := taskMeta["attached_files"].([]any); ok {
|
|
for _, f := range files {
|
|
filePath, ok := f.(string)
|
|
if !ok || filePath == "" {
|
|
continue
|
|
}
|
|
var fileSize int64
|
|
if info, err := os.Stat(filePath); err == nil {
|
|
fileSize = info.Size()
|
|
}
|
|
att := &store.TeamTaskAttachmentData{
|
|
TaskID: task.ID,
|
|
TeamID: team.ID,
|
|
ChatID: chatID,
|
|
Path: filePath,
|
|
FileSize: fileSize,
|
|
MimeType: mimeFromExt(filepath.Ext(filePath)),
|
|
CreatedByAgentID: &agentID,
|
|
}
|
|
if err := t.manager.Store().AttachFileToTask(ctx, att); err != nil {
|
|
slog.Warn("executeCreate: auto-attach media failed", "task_id", task.ID, "path", filePath, "error", err)
|
|
}
|
|
}
|
|
}
|
|
|
|
agentKey := t.manager.AgentKeyFromID(ctx, agentID)
|
|
t.manager.BroadcastTeamEvent(ctx, protocol.EventTeamTaskCreated, BuildTaskEventPayload(
|
|
team.ID.String(), task.ID.String(),
|
|
status,
|
|
"agent", agentKey,
|
|
WithSubject(subject),
|
|
WithContextInfo(ctx),
|
|
WithTimestamp(task.CreatedAt.UTC().Format("2006-01-02T15:04:05Z")),
|
|
))
|
|
// Track for post-turn dispatch. If no post-turn hook (e.g. HTTP API), dispatch immediately.
|
|
// Member requests with auto_dispatch=false stay pending for leader review — skip dispatch.
|
|
if status == store.TeamTaskStatusPending && !skipAutoDispatch {
|
|
if ptd := PendingTeamDispatchFromCtx(ctx); ptd != nil {
|
|
ptd.Add(team.ID, task.ID)
|
|
} else {
|
|
// Fallback: assign (pending → in_progress + lock) then dispatch.
|
|
if err := t.manager.Store().AssignTask(ctx, task.ID, assigneeID, team.ID); err != nil {
|
|
slog.Warn("executeCreate: fallback assign failed", "task_id", task.ID, "error", err)
|
|
} else {
|
|
t.manager.BroadcastTeamEvent(ctx, protocol.EventTeamTaskDispatched, BuildTaskEventPayload(
|
|
team.ID.String(), task.ID.String(),
|
|
store.TeamTaskStatusInProgress,
|
|
"system", "fallback_dispatch",
|
|
WithTaskInfo(task.TaskNumber, task.Subject),
|
|
WithOwnerAgentKey(t.manager.AgentKeyFromID(ctx, assigneeID)),
|
|
WithChannel(task.Channel),
|
|
WithChatID(task.ChatID),
|
|
WithPeerKind(ToolPeerKindFromCtx(ctx)),
|
|
WithLocalKey(ToolLocalKeyFromCtx(ctx)),
|
|
))
|
|
t.manager.DispatchTaskToAgent(ctx, task, team, assigneeID)
|
|
}
|
|
}
|
|
}
|
|
|
|
assigneeName := t.manager.AgentDisplayName(ctx, t.manager.AgentKeyFromID(ctx, assigneeID))
|
|
if assigneeName == "" {
|
|
assigneeName = t.manager.AgentKeyFromID(ctx, assigneeID)
|
|
}
|
|
msg := fmt.Sprintf("Task created: %s (id=%s, task_number=%d, status=%s, assignee=%s)", subject, task.ID, task.TaskNumber, status, assigneeName)
|
|
|
|
// Soft guardrail: warn if subject suggests multiple deliverables.
|
|
// Only checks subject (not description) — detailed descriptions are fine.
|
|
if subject != "" {
|
|
subjLower := strings.ToLower(subject)
|
|
hasCompound := strings.Contains(subjLower, " and ") &&
|
|
(strings.Contains(subjLower, "implement") || strings.Contains(subjLower, "create") ||
|
|
strings.Contains(subjLower, "build") || strings.Contains(subjLower, "design") ||
|
|
strings.Contains(subjLower, "write") || strings.Contains(subjLower, "develop"))
|
|
if hasCompound {
|
|
msg += "\n\nWarning: This task subject suggests multiple deliverables. Consider splitting into separate tasks if they need different skills."
|
|
}
|
|
}
|
|
return NewResult(msg)
|
|
}
|