mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-13 04:14:56 +00:00
dev
8
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 yatulandClaude Opus 5
|
305bd7c50d |
fix(teams): grant a delegated lead read access to its own team (#1537)
* 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>
|
||
|
viettranx
|
6a9b747b65 |
feat: propagate RBAC role through dispatch + bypass file-writer grant for admins
Dashboard users (and other tenant-authenticated admin/operator/owner
actors) dispatching team tasks to agents in Telegram/Discord groups
previously hit the empty-sender DENY rule when the assigned agent
tried to write files — because dashboard users have tenant identities
(e.g. "viettx") that don't match the numeric file-writer grants stored
per channel.
Two-part fix:
1. CheckFileWriterPermission / CheckCronPermission add an RBAC bypass
at the store layer: if ctx carries RoleFromContext in {admin,
operator, owner}, skip the per-user grant check. Tenant RBAC already
pre-authenticated these callers at the gateway edge; requiring a
redundant per-channel grant blocks legitimate admin work.
2. Role propagation through the full dispatch chain:
ctx (RoleFromContext)
→ RunRequest.Role
→ SubagentTask.OriginRole / AnnounceMetadata.OriginRole
/ DelegateRequest.Role
→ InboundMessage.Metadata[MetaOriginRole]
→ subagentAnnounceRouting.Role / teammateRole
→ RunRequest.Role (re-ingress)
→ store.WithRole(ctx, ...) in loop_context
Covers subagent announce, delegate announce, teammate dispatch
(WS dashboard, tool), session_send, and processNormalMessage's
synthetic-sender override path. team_tasks_create persists
origin_role for deferred dispatches.
Viewer and empty roles fall through to the existing sender-based path
(no bypass).
Regression tests:
- A.10 AdminRoleBypass: admin/operator/owner in ctx allowed even with
empty sender (normally DENY).
- A.11 ViewerRoleDoesNotBypass: viewer and empty roles still hit DENY.
Builds + integration + unit tests green.
|
||
|
viettranx
|
1bd90123c6 |
fix(gateway): override synthetic senders with MetaOriginSenderID + propagate through dashboard dispatch
Closes the two last unpropagated paths identified by the #915 e2e audit. processNormalMessage now prefers MetaOriginSenderID when the on-wire SenderID is a synthetic internal sender (notification:, ticker:, session_send_tool, system:escalation, etc.). Real user senders pass through unchanged. Defense-in-depth: MetaOriginSenderID is ignored if it itself looks synthetic, preventing a propagation bug from turning into a permission bypass. teams_tasks_mutations dashboard dispatch now injects MetaOriginSenderID into the teammate InboundMessage metadata, preferring the WS caller's sender (live dashboard) and falling back to origin_sender_id stored on the task (for deferred dispatches). Without this, dashboard-triggered team tasks in group chats would hit the empty-sender DENY rule. team_tasks_create persists origin_sender_id into task.Metadata at creation time so the deferred dashboard dispatch path can reconstruct the original actor identity. |
||
|
viettranx
|
9e5cec03ba |
feat: preserve acting sender + migrate ACTOR sites across tool chain
Resolves issue #915 (Telegram group write_file permission denied after /addwriter) by reconstructing the real sender on announce re-ingress and migrating ownership/audit call sites off the scope-coupled UserID. Propagation chain (new MetaOriginSenderID key): SubagentTask.OriginSenderID → AnnounceMetadata.OriginSenderID → InboundMessage.Metadata[MetaOriginSenderID] → subagentAnnounceRouting.SenderID → RunRequest.SenderID Same chain for delegate_tool.announceToParent, team_tool_dispatch, sessions_send, team_tasks_blocker escalation. ACTOR migration (owner/audit now uses ActorIDFromContext): - publish_skill: skill owner = individual sender - skill_manage create/patch/delete: owner = sender; ownership check accepts actor or legacy UserID for pre-migration rows - delegate_tool: DelegateRequest.UserID + DomainEvent.UserID = actor - team_tasks_blocker: blocker attribution + escalation = actor - team_tool_cache: team access-policy check = actor (fixes per-user allow/deny lists in group chats) Scope-intentional (unchanged, commented): cron.go, team_tasks_create.go. |
||
|
|
ad893908a5 |
fix(telegram): propagate local_key for forum topic routing in team notifications (#800)
* fix(telegram): propagate local_key for forum topic routing in team notifications
Team task status messages (dispatched, completed, progress) were always
delivered to the General topic in Telegram forum groups because the
notification pipeline had no access to the originating topic's local_key.
Root cause: wireTeamProgressNotifySubscriber in gateway_events.go published
OutboundMessage with no Metadata, so the Telegram adapter had no
message_thread_id to route to the correct forum topic.
Fix has two parts:
1. Team notify path (root cause):
- Add LocalKey field to TeamTaskEventPayload (protocol)
- Extract local_key from tool context in WithContextInfo()
- Add LocalKey to NotifyRoutingMeta
- Pass LocalKey through to OutboundMessage Metadata in both
leader mode (InboundMessage) and direct mode (OutboundMessage)
2. MCP bridge context (supporting):
- Propagate local_key and session_key through bridge HTTP headers
- Add X-Local-Key and X-Session-Key to BridgeContext
- Extract and inject into tool context in gateway middleware
- Include in HMAC signature for integrity
Closes #798
* fix(telegram): pass LocalKey from task metadata in all dispatch/fail broadcast sites
The initial fix added LocalKey to the event payload and WithContextInfo(),
but 4 broadcast call sites use individual With* options instead of
WithContextInfo — so LocalKey was never populated for:
- fallback_dispatch (team_tasks_create.go)
- dispatch_unblocked (team_tool_dispatch.go)
- post_turn dispatch (team_tool_validation.go)
- blocker/fail (team_tasks_blocker.go)
Add WithLocalKey() option function and extract TaskMetaLocalKey from task
metadata at each site, matching the existing TaskMetaPeerKind pattern.
* test(mcp): add HMAC verification tests for extra params (localKey, sessionKey)
- Add tests for SignBridgeContext/VerifyBridgeContext with extra params
- Test backward compat fallback for pre-localKey sessions
- Test that param order matters in signature
- Add clarifying comment for routing context injection security model
---------
Co-authored-by: Jens Henke <jens@henke.dk>
Co-authored-by: viettranx <viettranx@gmail.com>
|
||
|
viettranx
|
36b43ed7f1 |
feat(workspace): leader dual workspace with auto-copy to team on delegate
- Revert leader workspace override: leader keeps personal workspace as default, team workspace accessible via ToolTeamWorkspaceFromCtx - Auto-copy: when leader creates team_tasks, scan subject+description for file paths → copy from personal to team workspace so members can access them - Safety: Lstat (reject symlinks), 10MB size cap, .env excluded from allowed extensions, path traversal blocked - Prompt hint: clarify members can only access team workspace files, referenced files are auto-copied |
||
|
viettranx
|
014f74ec15 |
fix(agent): group session unresponsive during team task execution (#266)
Two fixes: 1. Remove assistant prefill from team task reminders. The injected [user]+[assistant]+[user] pattern caused LLMs to treat the canned ack as "turn complete", returning NO_REPLY for every user message in group sessions with active tasks. Reminders are now merged into the user message as prefix tags. 2. Add PeerKind propagation to team notification routing. TaskTicker and progress notifications were missing PeerKind on InboundMessage, causing them to route to phantom DM sessions instead of the correct group session. PeerKind is now carried through event payloads, notify queue metadata, and all inbound message publications. |
||
|
viettranx
|
78253f1841 |
refactor: decompose consumer, agent loop, team tools, and MCP manager
6-phase pure refactor — zero behavior change, all tests pass. Phase 1: DRY event payloads + metadata keys - BuildTaskEventPayload() helper with functional options replaces 24 inline constructions - 26 metadata string constants replace ~66 magic strings across dispatch/consumer Phase 2: Consumer handler decomposition - ConsumerDeps struct collapses 11-13 positional params to 3 per handler - Extract startTaskLockRenewal() and resolveTeamTaskOutcome() Phase 3: loop.go extraction (978→686 LOC) - enrichInputMedia(): media persistence, ref collection, tag enrichment - injectTeamTaskReminders(): leader/member task context injection - buildFilteredTools(): policy, bootstrap, channel, iteration filtering - collectRefsByKind() helper eliminates 3x copy-paste Phase 4: team_tasks_mutations split (630→318 LOC) - executeCreate extracted to team_tasks_create.go - resolveTeamAndTask() helper used by 13 execute* functions Phase 5: MCP LoadForAgent split (~90→~15 LOC body) - resolveServerCredentials() + connectAndFilter() helpers Phase 6: Comments + DI - 5 inaccurate comments fixed - ToolExecutor interface for dependency inversion (Loop.tools field) - FilterTools accepts ToolExecutor instead of concrete *Registry |