diff --git a/AGENTS.md b/AGENTS.md index 2a1b7a44..8a4daa4a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,6 +54,7 @@ make release ## Git And PRs - Use Conventional Commits for commit messages and PR titles. +- Codex-generated commit messages must use the `.github/workflows/pr-message.yml` title format: `[optional scope][optional !]: ` with an allowed lower-case type such as `feat`, `fix`, `docs`, `test`, `ci`, or `chore`. - Keep PR bodies compatible with `.github/workflows/pr-message.yml`: use concise GitHub Markdown, and do not include `Co-authored-by:` trailers or unresolved conflict markers. - Keep PR bodies short and focused on summary, validation, impact, related issues, and notes. - Check PR CI before requesting review or merge. diff --git a/internal/api/participant_test.go b/internal/api/participant_test.go index dcd18a72..0b176848 100644 --- a/internal/api/participant_test.go +++ b/internal/api/participant_test.go @@ -314,6 +314,61 @@ func TestParticipantMessageRouteCanonicalizesAgentIDAlias(t *testing.T) { } } +func TestParticipantDeleteWithAgentCleanupRemovesCSGClawUser(t *testing.T) { + agentSvc := mustNewService(t) + if _, err := agentSvc.Create(context.Background(), agent.CreateRequest{ + Spec: agent.CreateAgentSpec{ + ID: "u-qa", + Name: "qa", + Role: agent.RoleWorker, + RuntimeKind: agent.RuntimeKindPicoClawSandbox, + Image: "agent-image:test", + }, + }); err != nil { + t.Fatalf("seed agent: %v", err) + } + imSvc := im.NewServiceFromBootstrap(im.Bootstrap{ + CurrentUserID: "u-admin", + Users: []im.User{ + {ID: "u-admin", Name: "admin", Handle: "admin"}, + {ID: "u-qa", Name: "qa", Handle: "qa"}, + }, + }) + participantSvc := participant.NewService(participant.NewMemoryStore([]apitypes.Participant{{ + ID: "qa", + Channel: participant.ChannelCSGClaw, + Type: participant.TypeAgent, + Name: "qa", + ChannelUserRef: "u-qa", + ChannelUserKind: participant.ChannelUserKindLocalUserID, + AgentID: "u-qa", + LifecycleStatus: participant.LifecycleStatusActive, + Mentionable: true, + }}), participant.WithAgentService(agentSvc), participant.WithIMService(imSvc)) + srv := &Handler{ + svc: agentSvc, + im: imSvc, + participant: participantSvc, + } + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodDelete, "/api/v1/channels/csgclaw/participants/qa?delete_agent=if_unreferenced", nil) + srv.Routes().ServeHTTP(rec, req) + + if rec.Code != http.StatusNoContent { + t.Fatalf("status = %d, want %d; body=%s", rec.Code, http.StatusNoContent, rec.Body.String()) + } + if _, ok := participantSvc.Get(participant.ChannelCSGClaw, "qa"); ok { + t.Fatal("participant csgclaw:qa still exists after delete") + } + if _, ok := agentSvc.Agent("u-qa"); ok { + t.Fatal("agent u-qa still exists after delete") + } + if _, ok := imSvc.User("u-qa"); ok { + t.Fatal("user u-qa still exists after participant agent cleanup") + } +} + func TestParticipantNotificationRouteAcceptsNotificationParticipant(t *testing.T) { participantSvc := participant.NewService(participant.NewMemoryStore([]apitypes.Participant{{ ID: "alerts", diff --git a/internal/participant/service.go b/internal/participant/service.go index 152df44b..096200c7 100644 --- a/internal/participant/service.go +++ b/internal/participant/service.go @@ -381,10 +381,38 @@ func (s *Service) Delete(ctx context.Context, channel, id string, opts DeleteOpt if err := s.agents.Delete(ctx, deleted.AgentID); err != nil { return deleted, true, err } + if err := s.deleteUnreferencedCSGClawAgentUser(deleted); err != nil { + return deleted, true, err + } } return deleted, true, nil } +func (s *Service) deleteUnreferencedCSGClawAgentUser(deleted apitypes.Participant) error { + if s == nil || s.store == nil || s.im == nil { + return nil + } + if deleted.Channel != ChannelCSGClaw || deleted.Type != TypeAgent || deleted.ChannelUserKind != ChannelUserKindLocalUserID { + return nil + } + userID := strings.TrimSpace(deleted.ChannelUserRef) + if userID == "" { + return nil + } + for _, item := range s.store.List(ListOptions{Channel: ChannelCSGClaw}) { + if strings.TrimSpace(item.ChannelUserRef) == userID { + return nil + } + } + if err := s.im.DeleteUser(userID); err != nil { + if strings.Contains(err.Error(), "not found") { + return nil + } + return fmt.Errorf("delete CSGClaw user %q: %w", userID, err) + } + return nil +} + type normalizedCreateRequest struct { ID string Channel string diff --git a/internal/participant/service_test.go b/internal/participant/service_test.go index 3a411726..f2ebb7e2 100644 --- a/internal/participant/service_test.go +++ b/internal/participant/service_test.go @@ -396,6 +396,69 @@ func TestDeleteParticipantRejectsAgentCleanupWhenStillReferenced(t *testing.T) { } } +func TestDeleteParticipantAgentCleanupKeepsSharedCSGClawUser(t *testing.T) { + agentSvc := mustNewAgentService(t) + imSvc := im.NewServiceFromBootstrap(im.Bootstrap{ + CurrentUserID: "u-admin", + Users: []im.User{ + {ID: "u-admin", Name: "admin", Handle: "admin"}, + {ID: "u-qa", Name: "QA", Handle: "qa"}, + }, + }) + svc := NewService(NewMemoryStore(nil), WithAgentService(agentSvc), WithIMService(imSvc)) + if _, err := agentSvc.Create(context.Background(), agent.CreateRequest{ + Spec: agent.CreateAgentSpec{ + ID: "u-qa", + Name: "QA Runtime", + Role: agent.RoleWorker, + RuntimeKind: agent.RuntimeKindPicoClawSandbox, + Image: "agent-image:test", + }, + }); err != nil { + t.Fatalf("seed agent: %v", err) + } + for _, req := range []CreateRequest{ + { + ID: "qa", + Channel: ChannelCSGClaw, + Type: TypeAgent, + Name: "QA", + ChannelUser: ChannelUserSpec{ + Ref: "u-qa", + Kind: ChannelUserKindLocalUserID, + }, + AgentBinding: AgentBindingSpec{ + Mode: BindingModeReuse, + AgentID: "u-qa", + }, + }, + { + ID: "qa-human-ref", + Channel: ChannelCSGClaw, + Type: TypeHuman, + Name: "QA Human Ref", + ChannelUser: ChannelUserSpec{ + Ref: "u-qa", + Kind: ChannelUserKindLocalUserID, + }, + }, + } { + if _, err := svc.Create(context.Background(), req); err != nil { + t.Fatalf("Create(%s) error = %v", req.ID, err) + } + } + + if _, ok, err := svc.Delete(context.Background(), ChannelCSGClaw, "qa", DeleteOptions{DeleteAgent: DeleteAgentIfUnreferenced}); err != nil || !ok { + t.Fatalf("Delete() ok=%v error=%v, want ok", ok, err) + } + if _, exists := agentSvc.Agent("u-qa"); exists { + t.Fatal("agent u-qa still exists after cleanup") + } + if _, exists := imSvc.User("u-qa"); !exists { + t.Fatal("shared user u-qa was deleted despite another participant reference") + } +} + func TestCreateHumanParticipantRejectsCreateAgentBinding(t *testing.T) { svc := NewService(NewMemoryStore(nil)) diff --git a/web/app/src/api/agents.ts b/web/app/src/api/agents.ts index e3f9e1da..7fb4a683 100644 --- a/web/app/src/api/agents.ts +++ b/web/app/src/api/agents.ts @@ -24,6 +24,10 @@ export type FetchAgentLogsOptions = { lines?: number; }; +export type DeleteBotOptions = { + deleteAgent?: boolean; +}; + export type AgentUpdatePayload = { agent_profile?: JSONRecord; avatar?: string; @@ -178,8 +182,13 @@ export function patchNotificationBotRequest(botID: string, payload: CreateBotPay }).then(participantToAgentLike); } -export function deleteBotRequest(botID: string): Promise { - return del(`api/v1/channels/csgclaw/participants/${encodeURIComponent(botID)}`); +export function deleteBotRequest(botID: string, options: DeleteBotOptions = {}): Promise { + const params = new URLSearchParams(); + if (options.deleteAgent) { + params.set("delete_agent", "if_unreferenced"); + } + const query = params.toString(); + return del(`api/v1/channels/csgclaw/participants/${encodeURIComponent(botID)}${query ? `?${query}` : ""}`); } export function runAgentActionRequest(agentID: string, action: string): Promise { diff --git a/web/app/src/hooks/workspace/useAgentController.ts b/web/app/src/hooks/workspace/useAgentController.ts index 155cd5ec..b04671d2 100644 --- a/web/app/src/hooks/workspace/useAgentController.ts +++ b/web/app/src/hooks/workspace/useAgentController.ts @@ -953,7 +953,7 @@ export function useAgentController({ try { let updatedAgent: AgentLike | null = null; if (action === "delete") { - await deleteBotRequest(csgclawParticipantIDForAgent(item)); + await deleteBotRequest(csgclawParticipantIDForAgent(item), { deleteAgent: true }); } else { updatedAgent = await runAgentActionRequest(item.id, action); } diff --git a/web/app/tests/legacy-contract.test.ts b/web/app/tests/legacy-contract.test.ts index 666cae16..2c87f3fc 100644 --- a/web/app/tests/legacy-contract.test.ts +++ b/web/app/tests/legacy-contract.test.ts @@ -65,7 +65,8 @@ describe("legacy UI contract", () => { expect(source).toContain('"api/v1/channels/csgclaw/participants?type=notification"'); expect(source).toContain("patchNotificationBotRequest"); expect(source).toContain("createNotificationBotRequest"); - expect(source).toContain("del(`api/v1/channels/csgclaw/participants/${encodeURIComponent(botID)}`)"); + expect(source).toContain('params.set("delete_agent", "if_unreferenced");'); + expect(source).toContain("deleteBotRequest(csgclawParticipantIDForAgent(item), { deleteAgent: true })"); expect(source).toContain("const SHOW_AGENT_LIFECYCLE_ACTIONS = false;"); expect(source).toContain('export const BOT_TYPE_NOTIFICATION = "notification"'); expect(source).toContain("function NotifierControls");