Skip to content

Commit 2b57888

Browse files
authored
Merge pull request #3598 from codeeu/feature/support-role-remove
Support copilot: add user_role_remove
2 parents a3ac808 + 18c2615 commit 2b57888

10 files changed

Lines changed: 426 additions & 33 deletions

app/Jobs/Support/ExecuteApprovedSupportActionJob.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use App\Services\Support\UserProfileUpdateService;
1313
use App\Services\Support\UserRestoreService;
1414
use App\Services\Support\UserRoleAddService;
15+
use App\Services\Support\UserRoleRemoveService;
1516
use Illuminate\Bus\Queueable;
1617
use Illuminate\Contracts\Queue\ShouldQueue;
1718
use Illuminate\Foundation\Bus\Dispatchable;
@@ -35,6 +36,7 @@ public function handle(
3536
ArtisanCommandRunner $artisanRunner,
3637
ContentUpdateService $contentUpdate,
3738
UserRoleAddService $userRoleAdd,
39+
UserRoleRemoveService $userRoleRemove,
3840
): void
3941
{
4042
$approval = SupportApproval::findOrFail($this->supportApprovalId);
@@ -92,6 +94,8 @@ public function handle(
9294
} elseif ($action === 'user_role_add') {
9395
// Re-read the role + emails from the case email at execution time.
9496
$result = $userRoleAdd->addFromCase($case, dryRun: false, viaEmailApproval: true);
97+
} elseif ($action === 'user_role_remove') {
98+
$result = $userRoleRemove->removeFromCase($case, dryRun: false, viaEmailApproval: true);
9599
} elseif ($action === 'code_change') {
96100
$result = $cursorAgent->launchCodeAgent(
97101
prompt: (string) ($payload['cursor_prompt'] ?? ''),

app/Jobs/Support/ProcessSupportCaseDiagnosticsJob.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use App\Services\Support\UserProfileUpdateService;
1313
use App\Services\Support\UserRestoreService;
1414
use App\Services\Support\UserRoleAddService;
15+
use App\Services\Support\UserRoleRemoveService;
1516
use Illuminate\Bus\Queueable;
1617
use Illuminate\Contracts\Queue\ShouldQueue;
1718
use Illuminate\Foundation\Bus\Dispatchable;
@@ -32,6 +33,7 @@ public function handle(
3233
UserRestoreService $userRestore,
3334
UserProfileUpdateService $userProfileUpdate,
3435
UserRoleAddService $userRoleAdd,
36+
UserRoleRemoveService $userRoleRemove,
3537
ArtisanCommandRunner $artisanRunner,
3638
ContentUpdateService $contentUpdate,
3739
): void {
@@ -79,6 +81,20 @@ public function handle(
7981
);
8082
}
8183

84+
if ($case->case_type === 'role_remove') {
85+
$dryRunResult = $userRoleRemove->removeFromCase($case, dryRun: true);
86+
$logger->log(
87+
case: $case,
88+
actionName: 'user_role_remove',
89+
actionType: 'write',
90+
input: ['dry_run' => true],
91+
output: $dryRunResult,
92+
succeeded: (bool) ($dryRunResult['ok'] ?? false),
93+
executedBy: 'agent',
94+
correlationId: $case->correlation_id,
95+
);
96+
}
97+
8298
if ($case->case_type === 'role_add') {
8399
$dryRunResult = $userRoleAdd->addFromCase($case, dryRun: true);
84100
$logger->log(

app/Services/Support/Agents/CursorCliTriageProvider.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ class CursorCliTriageProvider implements TriageProvider
2828
'certificate_issue',
2929
'role_issue',
3030
'role_add',
31+
'role_remove',
3132
'code_change',
3233
'artisan_command',
3334
'content_update',
@@ -138,6 +139,9 @@ private function buildPrompt(SupportCase $case, string $rawText): string
138139
users identified by email. Put the affected emails in target_email/secondary_emails, put the
139140
role being granted in "role_name" (singular, e.g. "leading teacher"), and set "role_operation"
140141
to "add". Include EVERY email listed in the request (one per person), even for long pasted tables.
142+
Use "role_remove" when the request is to remove/revoke/delete a role (e.g. "ambassador") from one
143+
or more users. Use the same target_email/secondary_emails and role_name fields, and set
144+
"role_operation" to "remove".
141145
{$artisanBlock}{$contentBlock}
142146
JSON schema to return:
143147
{
@@ -151,8 +155,8 @@ private function buildPrompt(SupportCase $case, string $rawText): string
151155
"reasoning_summary": "<one sentence>",
152156
"profile_firstname": "<requested first name or null>",
153157
"profile_lastname": "<requested last name or null>",
154-
"role_name": "<for role_add: the role to grant, e.g. leading teacher, else null>",
155-
"role_operation": "<for role_add: add (remove is not supported), else null>",
158+
"role_name": "<for role_add/role_remove: the role affected, else null>",
159+
"role_operation": "<for role_add: add; for role_remove: remove; else null>",
156160
"change_summary": "<for code_change: one sentence describing the fix, else null>",
157161
"change_area": "<for code_change: e.g. frontend/blade/css/js, else null>",
158162
"cursor_prompt": "<for code_change: a precise instruction for a coding agent to implement the fix and open a PR, else null>",
@@ -296,6 +300,7 @@ private function normalize(array $data): array
296300
'profile_update' => 'user_profile_update',
297301
'account_restore' => 'user_restore',
298302
'role_add' => 'user_role_add',
303+
'role_remove' => 'user_role_remove',
299304
'code_change' => 'code_change',
300305
'artisan_command' => 'artisan_command',
301306
'content_update' => 'content_update',
@@ -352,7 +357,11 @@ private function normalizeRoleOperation(mixed $value): ?string
352357
}
353358
$value = strtolower(trim($value));
354359

355-
return in_array($value, ['add', 'grant', 'assign'], true) ? 'add' : null;
360+
return match (true) {
361+
in_array($value, ['add', 'grant', 'assign'], true) => 'add',
362+
in_array($value, ['remove', 'revoke', 'delete'], true) => 'remove',
363+
default => null,
364+
};
356365
}
357366

358367
private function stringOrNull(mixed $value): ?string

app/Services/Support/Agents/TriageAgentService.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,21 @@ private function heuristicTriage(SupportCase $case): array
5959
$text = Str::lower($rawText);
6060
$profile = $this->profileParser->parse($rawText);
6161
$roleRequest = $this->roleParser->parse($rawText);
62-
$hasRoleRequest = $roleRequest['role'] !== null
62+
$hasRoleAddRequest = $roleRequest['role'] !== null
6363
&& $roleRequest['operation'] === 'add'
6464
&& $roleRequest['emails'] !== [];
65+
$hasRoleRemoveRequest = $roleRequest['role'] !== null
66+
&& $roleRequest['operation'] === 'remove'
67+
&& $roleRequest['emails'] !== [];
68+
$hasRoleRequest = $hasRoleAddRequest || $hasRoleRemoveRequest;
6569

6670
// V1 heuristic placeholder (replace with LLM later, keep output schema stable).
6771
$caseType = 'unknown';
6872
$runbook = 'unknown';
69-
if ($hasRoleRequest) {
73+
if ($hasRoleRemoveRequest) {
74+
$caseType = 'role_remove';
75+
$runbook = 'remove_user_role';
76+
} elseif ($hasRoleAddRequest) {
7077
$caseType = 'role_add';
7178
$runbook = 'add_user_role';
7279
} elseif (Str::contains($text, ['soft-deleted', 'deleted', 'restore account', 'account missing'])) {
@@ -120,6 +127,7 @@ private function heuristicTriage(SupportCase $case): array
120127
$requestedAction = match ($caseType) {
121128
'profile_update' => 'user_profile_update',
122129
'role_add' => 'user_role_add',
130+
'role_remove' => 'user_role_remove',
123131
default => null,
124132
};
125133

app/Services/Support/SupportApprovalEmailService.php

Lines changed: 74 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public function approvalSubject(SupportCase $case): string
2626
'profile_update' => 'Please review — name change',
2727
'account_restore' => 'Please review — account restore',
2828
'role_add' => 'Please review — add user role',
29+
'role_remove' => 'Please review — remove user role',
2930
'code_change' => 'Please review — proposed code fix (PR into dev)',
3031
'artisan_command' => 'Please review — proposed server maintenance command',
3132
'content_update' => 'Please review — proposed content/copy change',
@@ -282,6 +283,20 @@ private function proposedActionForCase(SupportCase $case): array
282283
}
283284
}
284285

286+
if ($case->case_type === 'role_remove') {
287+
$role = $this->roleResolver->resolve($case);
288+
if ($role['role'] !== null && $role['emails'] !== []) {
289+
return [
290+
'action' => 'user_role_remove',
291+
'payload' => [
292+
'operation' => $role['operation'],
293+
'role' => $role['role'],
294+
'emails' => $role['emails'],
295+
],
296+
];
297+
}
298+
}
299+
285300
if ($case->case_type === 'role_add') {
286301
$role = $this->roleResolver->resolve($case);
287302
if ($role['role'] !== null && $role['emails'] !== []) {
@@ -514,8 +529,8 @@ private function dryRunPlannedChangeLines(SupportCase $case, array $proposedActi
514529
return $this->dryRunContentLines($case);
515530
}
516531

517-
if ($action === 'user_role_add') {
518-
return $this->dryRunRoleAddLines($case, $payload);
532+
if ($action === 'user_role_add' || $action === 'user_role_remove') {
533+
return $this->dryRunRoleChangeLines($case, $payload, $action);
519534
}
520535

521536
return [
@@ -529,10 +544,13 @@ private function dryRunPlannedChangeLines(SupportCase $case, array $proposedActi
529544
* @param array<string, mixed> $payload
530545
* @return list<string>
531546
*/
532-
private function dryRunRoleAddLines(SupportCase $case, array $payload): array
547+
private function dryRunRoleChangeLines(SupportCase $case, array $payload, string $action): array
533548
{
549+
$actionName = $action === 'user_role_remove' ? 'user_role_remove' : 'user_role_add';
550+
$isRemove = $action === 'user_role_remove';
551+
534552
$output = $case->actions()
535-
->where('action_name', 'user_role_add')
553+
->where('action_name', $actionName)
536554
->where('action_type', 'write')
537555
->latest()
538556
->first()?->output_json;
@@ -544,7 +562,9 @@ private function dryRunRoleAddLines(SupportCase $case, array $payload): array
544562

545563
$lines = [
546564
'',
547-
'We will add the role "'.$role.'" to the following CodeWeek accounts:',
565+
$isRemove
566+
? 'We will remove the role "'.$role.'" from the following CodeWeek accounts:'
567+
: 'We will add the role "'.$role.'" to the following CodeWeek accounts:',
548568
'',
549569
];
550570

@@ -554,22 +574,26 @@ private function dryRunRoleAddLines(SupportCase $case, array $payload): array
554574
$lines[] = ''.$email;
555575
}
556576
$lines[] = '';
557-
$lines[] = 'We will add the role to each account above that does not already have it.';
577+
$lines[] = $isRemove
578+
? 'We will remove the role from each account above that currently has it.'
579+
: 'We will add the role to each account above that does not already have it.';
558580

559581
return $lines;
560582
}
561583

562584
foreach ($items as $item) {
563-
$lines[] = ''.$this->roleItemLine((array) $item);
585+
$lines[] = ''.$this->roleItemLine((array) $item, $isRemove);
564586
}
565587

566588
$lines[] = '';
567-
$lines[] = 'Summary: '.$this->roleSummaryLine($summary);
589+
$lines[] = 'Summary: '.$this->roleSummaryLine($summary, $isRemove);
568590

569591
if (($summary['ambiguous'] ?? 0) > 0 || ($summary['user_not_found'] ?? 0) > 0) {
570592
$lines[] = '';
571593
$lines[] = 'Accounts marked "not found" or "needs manual check" will be skipped.';
572-
$lines[] = 'Approving will still add the role to all accounts that are ready.';
594+
$lines[] = $isRemove
595+
? 'Approving will still remove the role from all accounts that are ready.'
596+
: 'Approving will still add the role to all accounts that are ready.';
573597
}
574598

575599
return $lines;
@@ -578,7 +602,7 @@ private function dryRunRoleAddLines(SupportCase $case, array $payload): array
578602
/**
579603
* @param array<string, mixed> $item
580604
*/
581-
private function roleItemLine(array $item): string
605+
private function roleItemLine(array $item, bool $isRemove = false): string
582606
{
583607
$email = (string) ($item['email'] ?? '');
584608
$status = (string) ($item['status'] ?? '');
@@ -587,6 +611,9 @@ private function roleItemLine(array $item): string
587611
'would_add' => 'will be added',
588612
'added' => 'role added',
589613
'already_has_role' => 'already has this role (no change)',
614+
'would_remove' => 'will be removed',
615+
'removed' => 'role removed',
616+
'does_not_have_role' => 'does not have this role (no change)',
590617
'user_not_found' => 'no CodeWeek account found — skipped',
591618
'ambiguous' => 'multiple accounts match — needs manual check, skipped',
592619
default => $status,
@@ -598,12 +625,20 @@ private function roleItemLine(array $item): string
598625
/**
599626
* @param array<string, mixed> $summary
600627
*/
601-
private function roleSummaryLine(array $summary): string
628+
private function roleSummaryLine(array $summary, bool $isRemove = false): string
602629
{
603-
$toAdd = (int) (($summary['would_add'] ?? 0) + ($summary['added'] ?? 0));
604-
$parts = [$toAdd.' to add'];
605-
if (($summary['already_has_role'] ?? 0) > 0) {
606-
$parts[] = (int) $summary['already_has_role'].' already have it';
630+
if ($isRemove) {
631+
$toChange = (int) (($summary['would_remove'] ?? 0) + ($summary['removed'] ?? 0));
632+
$parts = [$toChange.' to remove'];
633+
if (($summary['does_not_have_role'] ?? 0) > 0) {
634+
$parts[] = (int) $summary['does_not_have_role'].' do not have it';
635+
}
636+
} else {
637+
$toChange = (int) (($summary['would_add'] ?? 0) + ($summary['added'] ?? 0));
638+
$parts = [$toChange.' to add'];
639+
if (($summary['already_has_role'] ?? 0) > 0) {
640+
$parts[] = (int) $summary['already_has_role'].' already have it';
641+
}
607642
}
608643
if (($summary['user_not_found'] ?? 0) > 0) {
609644
$parts[] = (int) $summary['user_not_found'].' not found';
@@ -796,6 +831,7 @@ private function completionHeadline(SupportCase $case, string $action, bool $suc
796831
'user_profile_update' => 'Done — name updated on CodeWeek account',
797832
'user_restore' => 'Done — CodeWeek account reactivated',
798833
'user_role_add' => 'Done — user role added',
834+
'user_role_remove' => 'Done — user role removed',
799835
'code_change' => 'Started — AI coding agent is preparing a PR into dev',
800836
'artisan_command' => 'Done — maintenance command completed on the server',
801837
'content_update' => 'Done — content updated',
@@ -840,8 +876,8 @@ private function buildCompletionBody(
840876
$lines = array_merge($lines, $this->completionFailureLines($action, $result, $email, $case->id));
841877
}
842878

843-
// Role-add is a batch action; the per-account list is shown above, so skip the single email line.
844-
if ($email !== '' && $action !== 'user_role_add') {
879+
// Role add/remove are batch actions; the per-account list is shown above.
880+
if ($email !== '' && ! in_array($action, ['user_role_add', 'user_role_remove'], true)) {
845881
$lines[] = '';
846882
$lines[] = 'Account email: '.$email;
847883
}
@@ -852,10 +888,10 @@ private function buildCompletionBody(
852888

853889
$lines[] = '';
854890
if ($succeeded) {
855-
$lines[] = $action === 'user_role_add'
891+
$lines[] = in_array($action, ['user_role_add', 'user_role_remove'], true)
856892
? 'No further action is needed. You do not need to reply to this email.'
857893
: 'No further action is needed. The supporter can sign in with their usual email and password.';
858-
if ($action !== 'user_role_add') {
894+
if (! in_array($action, ['user_role_add', 'user_role_remove'], true)) {
859895
$lines[] = 'You do not need to reply to this email.';
860896
}
861897
} else {
@@ -978,8 +1014,8 @@ private function completionSuccessLines(SupportCase $case, string $action, array
9781014
return $lines;
9791015
}
9801016

981-
if ($action === 'user_role_add') {
982-
return $this->completionRoleAddLines($inner);
1017+
if ($action === 'user_role_add' || $action === 'user_role_remove') {
1018+
return $this->completionRoleChangeLines($inner, $action);
9831019
}
9841020

9851021
return [
@@ -991,23 +1027,32 @@ private function completionSuccessLines(SupportCase $case, string $action, array
9911027
* @param array<string, mixed> $inner
9921028
* @return list<string>
9931029
*/
994-
private function completionRoleAddLines(array $inner): array
1030+
private function completionRoleChangeLines(array $inner, string $action): array
9951031
{
1032+
$isRemove = $action === 'user_role_remove';
9961033
$role = (string) ($inner['role'] ?? 'the requested role');
9971034
$items = is_array($inner['items'] ?? null) ? $inner['items'] : [];
9981035
$summary = is_array($inner['summary'] ?? null) ? $inner['summary'] : [];
999-
$addedCount = (int) ($summary['added'] ?? 0);
1036+
$changedCount = (int) ($isRemove ? ($summary['removed'] ?? 0) : ($summary['added'] ?? 0));
10001037

1001-
$lines = [
1002-
$addedCount === 1
1038+
if ($isRemove) {
1039+
$headline = $changedCount === 1
1040+
? 'We removed the role "'.$role.'" from 1 CodeWeek account.'
1041+
: 'We removed the role "'.$role.'" from '.$changedCount.' CodeWeek accounts.';
1042+
} else {
1043+
$headline = $changedCount === 1
10031044
? 'We added the role "'.$role.'" to 1 CodeWeek account.'
1004-
: 'We added the role "'.$role.'" to '.$addedCount.' CodeWeek accounts.',
1045+
: 'We added the role "'.$role.'" to '.$changedCount.' CodeWeek accounts.';
1046+
}
1047+
1048+
$lines = [
1049+
$headline,
10051050
'',
10061051
'Details:',
10071052
];
10081053

10091054
foreach ($items as $item) {
1010-
$lines[] = ''.$this->roleItemLine((array) $item);
1055+
$lines[] = ''.$this->roleItemLine((array) $item, $isRemove);
10111056
}
10121057

10131058
$skipped = (int) ($summary['user_not_found'] ?? 0) + (int) ($summary['ambiguous'] ?? 0);
@@ -1104,9 +1149,10 @@ private function humanizeError(string $code, string $action, int $caseId): strin
11041149
str_contains($code, 'no_effective_change') => 'The content already matched the requested text — no change was needed.',
11051150
str_starts_with($code, 'role_not_found') => 'We could not find a matching role. Check the role name (e.g. "leading teacher").',
11061151
str_starts_with($code, 'role_not_allowed') => 'That role is not on the list the copilot is allowed to add.',
1107-
str_contains($code, 'no_role_specified') => 'The request did not say which role to add.',
1152+
str_contains($code, 'no_role_specified') => 'The request did not say which role to change.',
11081153
str_contains($code, 'no_target_emails') => 'The request did not include any valid email addresses.',
11091154
str_contains($code, 'only_add_operation') => 'Only adding roles is supported right now (not removing).',
1155+
str_contains($code, 'only_remove_operation') => 'Only removing roles is supported for this action.',
11101156
$action === 'user_restore' && str_contains($code, 'verification') => 'The account was changed but we could not confirm it is fully active. Please verify in Nova.',
11111157
default => 'Technical detail: '.$code,
11121158
};

app/Services/Support/SupportRoleRequestParser.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ private function parseLabelled(string $text): array
6161
} elseif (preg_match('/(?:^|\n)\s*(?:remove|revoke)\s+role\s*[:\-]\s*([^\n\r]+)/iu', $text, $m)) {
6262
$role = $this->cleanRole($m[1]);
6363
$operation = 'remove';
64+
} elseif (preg_match('/\b(?:remove|revoke|delete)\s+(?:the\s+)?["\']?([^"\'\n\r]+?)["\']?\s+role\b/iu', $text, $m)) {
65+
$role = $this->cleanRole($m[1]);
66+
$operation = 'remove';
67+
} elseif (preg_match('/\b(?:remove|revoke|delete)\s+the\s+role\s+["\']?([^"\'\n\r]+?)["\']?\s+from\b/iu', $text, $m)) {
68+
$role = $this->cleanRole($m[1]);
69+
$operation = 'remove';
6470
} elseif (preg_match('/(?:^|\n)\s*role\s*(?:to\s+(add|remove))?\s*[:\-]\s*([^\n\r]+)/iu', $text, $m)) {
6571
$operation = strtolower(trim($m[1] ?? '')) === 'remove' ? 'remove' : 'add';
6672
$role = $this->cleanRole($m[2]);

0 commit comments

Comments
 (0)