From c7463fa87ba97e3e94ceb16cf9284b727fb67838 Mon Sep 17 00:00:00 2001 From: richardkeep Date: Tue, 27 Oct 2026 20:15:29 +0300 Subject: [PATCH 1/8] Add reply recipient picker --- app/Actions/CreateCommit.php | 7 +- .../Controllers/CommitReplyController.php | 2 +- .../Requests/CreateCommitReplyRequest.php | 20 ++++ resources/js/components/commit-thread.tsx | 40 +++++++- .../js/components/reply-recipient-picker.tsx | 96 +++++++++++++++++++ .../Controllers/CommitReplyControllerTest.php | 35 +++++++ 6 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 resources/js/components/reply-recipient-picker.tsx diff --git a/app/Actions/CreateCommit.php b/app/Actions/CreateCommit.php index fe8a0259e..00d84b8e9 100644 --- a/app/Actions/CreateCommit.php +++ b/app/Actions/CreateCommit.php @@ -51,8 +51,9 @@ public function __construct( * * @param list $images * @param array $imageDescriptions + * @param array $replyRecipients */ - public function handle(User $user, string $body, ?Commit $parent = null, ?Commit $quotedCommit = null, array $images = [], array $imageDescriptions = [], bool|string $linkPreview = true, ?NewPoll $poll = null): Commit + public function handle(User $user, string $body, ?Commit $parent = null, ?Commit $quotedCommit = null, array $images = [], array $imageDescriptions = [], bool|string $linkPreview = true, ?NewPoll $poll = null, array $replyRecipients = []): Commit { $url = match ($linkPreview) { true => $this->urls->first($body), @@ -60,7 +61,7 @@ public function handle(User $user, string $body, ?Commit $parent = null, ?Commit default => $linkPreview, }; - return DB::transaction(function () use ($user, $body, $parent, $quotedCommit, $images, $imageDescriptions, $url, $poll): Commit { + return DB::transaction(function () use ($user, $body, $parent, $quotedCommit, $images, $imageDescriptions, $url, $poll, $replyRecipients): Commit { $linkPreview = $this->linkPreviewFor($url); $createdAt = now(); $hasGoodReputation = $user->hasGoodReputation(); @@ -68,7 +69,7 @@ public function handle(User $user, string $body, ?Commit $parent = null, ?Commit $commit = $user->commits()->create([ 'body' => $this->emojiShortcodes->replace($body), - 'mentions' => $mentions, + 'mentions' => [...$replyRecipients, ...$mentions], 'parent_id' => $parent?->id, 'root_id' => $parent?->threadRootId(), 'status' => $user->isAdmin() || $hasGoodReputation ? CommitStatus::Approved : CommitStatus::Pending, diff --git a/app/Http/Controllers/CommitReplyController.php b/app/Http/Controllers/CommitReplyController.php index e7da33c70..64cefced5 100644 --- a/app/Http/Controllers/CommitReplyController.php +++ b/app/Http/Controllers/CommitReplyController.php @@ -26,7 +26,7 @@ public function index(ListCommitRepliesRequest $request, #[CurrentUser] ?User $u public function store(CreateCommitReplyRequest $request, #[CurrentUser] User $user, Commit $commit, CreateCommit $action, CommitThreadQuery $thread): JsonResponse { - $reply = $action->handle($user, $request->string('body')->value(), $commit, images: $request->images(), imageDescriptions: $request->imageDescriptions(), linkPreview: $request->linkPreview()); + $reply = $action->handle($user, $request->string('body')->value(), $commit, images: $request->images(), imageDescriptions: $request->imageDescriptions(), linkPreview: $request->linkPreview(), replyRecipients: $request->replyRecipients()); $parent = $thread->get($commit, $user); return response()->json([ diff --git a/app/Http/Requests/CreateCommitReplyRequest.php b/app/Http/Requests/CreateCommitReplyRequest.php index da3487990..4abd6f267 100644 --- a/app/Http/Requests/CreateCommitReplyRequest.php +++ b/app/Http/Requests/CreateCommitReplyRequest.php @@ -43,8 +43,12 @@ public function prepareForValidation(): void public function rules(): array { $body = $this->input('body'); + $parent = $this->route('commit'); + assert($parent instanceof Commit); return [ + 'reply_recipients' => ['sometimes', 'list'], + 'reply_recipients.*' => ['required', 'string', 'distinct', Rule::in(array_values($parent->mentions ?? []))], 'body' => CreateCommitRequest::bodyRules($this->hasFile('images')), 'link_preview_url' => ['nullable', 'string', Rule::in(is_string($body) ? resolve(Urls::class)->all($body) : [])], 'images' => ['nullable', 'list', 'max:'.CreateCommitImages::MAX_IMAGES], @@ -103,6 +107,22 @@ public function images(): array return is_array($images) ? array_values($images) : []; } + /** + * Only people already addressed by the parent may be included without an explicit body mention. + * + * @return array + */ + public function replyRecipients(): array + { + $parent = $this->route('commit'); + assert($parent instanceof Commit); + + /** @var list $recipients */ + $recipients = $this->validated('reply_recipients', []); + + return array_intersect($parent->mentions ?? [], $recipients); + } + /** * @return list */ diff --git a/resources/js/components/commit-thread.tsx b/resources/js/components/commit-thread.tsx index 6ac816c3d..0f3f36e3a 100644 --- a/resources/js/components/commit-thread.tsx +++ b/resources/js/components/commit-thread.tsx @@ -54,6 +54,7 @@ import InputError from '@/components/input-error'; import { LinkCardPicker } from '@/components/link-card-picker'; import { LinkPreview } from '@/components/link-preview'; import { MentionTextarea } from '@/components/mention-textarea'; +import { ReplyRecipientPicker } from '@/components/reply-recipient-picker'; import { StashLabel } from '@/components/stash-label'; import { showTimeout, timeoutFrom } from '@/components/timeout-dialog'; import { TimestampTooltip } from '@/components/timestamp-tooltip'; @@ -1344,6 +1345,22 @@ function ReplyComposer({ }) { const { user } = useUser(); const commitNotFound = useCommitNotFound(); + const [excludedRecipientIds, setExcludedRecipientIds] = useState( + [], + ); + const participants = Array.from( + new Map( + [ + [parent.user.id, parent.user.handle], + ...Object.entries(parent.mentions ?? {}).map(([handle, id]) => [ + id, + handle, + ]), + ] + .filter(([id]) => id !== user.id) + .map(([id, handle]) => [id, { id, handle }]), + ).values(), + ); const { body, images, @@ -1367,6 +1384,7 @@ function ReplyComposer({ body: string; images: File[]; image_descriptions: string[]; + reply_recipients: string[]; link_preview_url?: string; commit_rules_accepted?: boolean; }, @@ -1375,6 +1393,7 @@ function ReplyComposer({ body: '', images: [], image_descriptions: [], + reply_recipients: [], }); const textareaRef = useRef(null); const commitRules = useCommitRules(); @@ -1435,6 +1454,13 @@ function ReplyComposer({ body, images, image_descriptions: descriptions, + reply_recipients: participants + .filter( + ({ id }) => + id !== parent.user.id && + !excludedRecipientIds.includes(id), + ) + .map(({ id }) => id), commit_rules_accepted: commitRules.hasAccepted(), ...(linkCard.isChosen ? { link_preview_url: linkCard.link ?? '' } @@ -1484,6 +1510,14 @@ function ReplyComposer({ + {participants.length > 1 && ( + + )}
1 + ? 'Post your reply' + : `Reply to @${parent.user.handle}` + } className={cn( 'col-start-1 row-start-1 block field-sizing-content max-h-[50dvh] min-h-9 w-full resize-none overflow-x-hidden overflow-y-auto bg-transparent px-1 text-base leading-relaxed outline-none placeholder:text-muted-foreground focus-visible:outline-none md:text-[15px]', { invisible: preview.isPreviewing }, diff --git a/resources/js/components/reply-recipient-picker.tsx b/resources/js/components/reply-recipient-picker.tsx new file mode 100644 index 000000000..ecf069914 --- /dev/null +++ b/resources/js/components/reply-recipient-picker.tsx @@ -0,0 +1,96 @@ +import { useState } from 'react'; +import { Button } from '@/components/ui/button'; +import { Checkbox } from '@/components/ui/checkbox'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog'; + +export type ReplyParticipant = { id: string; handle: string }; + +export function ReplyRecipientPicker({ + participants, + authorId, + excludedIds, + onExcludedIdsChange, +}: { + participants: ReplyParticipant[]; + authorId: string; + excludedIds: string[]; + onExcludedIdsChange: (ids: string[]) => void; +}) { + const [open, setOpen] = useState(false); + const selected = participants.filter(({ id }) => !excludedIds.includes(id)); + + return ( + <> + + + + + Replying to + + Choose who to include. The commit author stays + selected. Anyone you mention in your reply will also + be notified. + + +
+ {participants.map(({ id, handle }) => ( + + ))} +
+ + + + + ); +} diff --git a/tests/Feature/Controllers/CommitReplyControllerTest.php b/tests/Feature/Controllers/CommitReplyControllerTest.php index 2e9b9fedc..d6ab1fa37 100644 --- a/tests/Feature/Controllers/CommitReplyControllerTest.php +++ b/tests/Feature/Controllers/CommitReplyControllerTest.php @@ -19,6 +19,41 @@ use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Storage; +it('only notifies selected conversation recipients and keeps explicit mentions', function (bool $selected, bool $explicit): void { + Queue::fake(); + config(['app.admins' => ['alice']]); + $author = User::factory()->create(['handle' => 'alice']); + $participant = User::factory()->create(['handle' => 'participant']); + $parent = Commit::factory()->create(['mentions' => ['participant' => $participant->id]]); + + $response = $this->actingAs($author)->postJson(route('commit-reply.store', $parent), [ + 'body' => $explicit ? 'Hello @participant' : 'Hello everyone', + 'reply_recipients' => $selected ? [$participant->id] : [], + ])->assertCreated(); + + $reply = Commit::query()->findOrFail($response->json('committed.id')); + expect($reply->mentions ?? [])->toBe($selected || $explicit ? ['participant' => $participant->id] : []); + expect($participant->notifications()->where('commit_id', $reply->id)->count())->toBe($selected || $explicit ? 1 : 0); + expect($parent->user->notifications()->where('commit_id', $reply->id)->count())->toBe(1); +})->with([ + 'selected' => [true, false], + 'unchecked' => [false, false], + 'explicit mention overrides unchecked' => [false, true], +]); + +it('rejects reply recipients outside the conversation', function (): void { + $parent = Commit::factory()->create(); + $stranger = User::factory()->create(); + + $this->actingAs(User::factory()->create()) + ->postJson(route('commit-reply.store', $parent), [ + 'body' => 'Hello everyone', + 'reply_recipients' => [$stranger->id], + ]) + ->assertUnprocessable() + ->assertJsonValidationErrors('reply_recipients.0'); +}); + it('answers a casual public Tom mention in the same thread', function (): void { config(['app.admins' => ['alice', 'tom'], 'ai.providers.opencode.key' => 'test', 'ai.providers.openai.key' => 'test']); Queue::fake([ReplyAsTom::class, DetectCommitLanguage::class]); From 9c04d006d233660e6037217617a18e18d16c9b02 Mon Sep 17 00:00:00 2001 From: richardkeep Date: Wed, 28 Oct 2026 06:53:42 +0300 Subject: [PATCH 2/8] Apply CodeRabbit suggestions fixes --- .../Controllers/CommitReplyController.php | 1 + .../Requests/CreateCommitReplyRequest.php | 26 ++++++++- resources/js/components/commit-thread.tsx | 14 +++-- .../js/components/reply-recipient-picker.tsx | 15 +++--- .../Controllers/CommitReplyControllerTest.php | 30 +++++++++++ tests/Unit/CommitThread.test.tsx | 53 +++++++++++++++++++ 6 files changed, 129 insertions(+), 10 deletions(-) diff --git a/app/Http/Controllers/CommitReplyController.php b/app/Http/Controllers/CommitReplyController.php index 64cefced5..7cc5bcc73 100644 --- a/app/Http/Controllers/CommitReplyController.php +++ b/app/Http/Controllers/CommitReplyController.php @@ -24,6 +24,7 @@ public function index(ListCommitRepliesRequest $request, #[CurrentUser] ?User $u return response()->json($replies->paginate($commit, $user, $request->boolean('newest'), $request->integer('limit', CommitRepliesQuery::PER_PAGE), $request->exceptedReplyId())); } + /** Creates a reply addressed to the parent author and the validated selection of conversation participants. */ public function store(CreateCommitReplyRequest $request, #[CurrentUser] User $user, Commit $commit, CreateCommit $action, CommitThreadQuery $thread): JsonResponse { $reply = $action->handle($user, $request->string('body')->value(), $commit, images: $request->images(), imageDescriptions: $request->imageDescriptions(), linkPreview: $request->linkPreview(), replyRecipients: $request->replyRecipients()); diff --git a/app/Http/Requests/CreateCommitReplyRequest.php b/app/Http/Requests/CreateCommitReplyRequest.php index 4abd6f267..d862583ed 100644 --- a/app/Http/Requests/CreateCommitReplyRequest.php +++ b/app/Http/Requests/CreateCommitReplyRequest.php @@ -7,6 +7,7 @@ use App\Actions\CreateCommitImages; use App\Models\Commit; use App\Models\User; +use App\Queries\MentionedUsersQuery; use App\Services\Urls; use Illuminate\Auth\Access\Response; use Illuminate\Contracts\Validation\ValidationRule; @@ -16,9 +17,12 @@ use Illuminate\Validation\Rules\Dimensions; use Illuminate\Validation\Rules\File; use Illuminate\Validation\Rules\In; +use Illuminate\Validation\Validator; final class CreateCommitReplyRequest extends FormRequest { + public const int MAX_REPLY_RECIPIENTS = 100; + public function authorize(): Response { $user = $this->user(); @@ -47,7 +51,7 @@ public function rules(): array assert($parent instanceof Commit); return [ - 'reply_recipients' => ['sometimes', 'list'], + 'reply_recipients' => ['sometimes', 'list', 'max:'.self::MAX_REPLY_RECIPIENTS], 'reply_recipients.*' => ['required', 'string', 'distinct', Rule::in(array_values($parent->mentions ?? []))], 'body' => CreateCommitRequest::bodyRules($this->hasFile('images')), 'link_preview_url' => ['nullable', 'string', Rule::in(is_string($body) ? resolve(Urls::class)->all($body) : [])], @@ -65,6 +69,26 @@ public function rules(): array ]; } + /** + * Bound the combined inherited and explicit recipients so reply chains cannot grow notification fanout indefinitely. + * + * @return array + */ + public function after(): array + { + return [function (Validator $validator): void { + if ($validator->errors()->isNotEmpty()) { + return; + } + + $recipients = [...$this->replyRecipients(), ...resolve(MentionedUsersQuery::class)->get($this->string('body')->value())]; + + if (count(array_unique($recipients)) > self::MAX_REPLY_RECIPIENTS) { + $validator->errors()->add('reply_recipients', 'A reply may include at most '.self::MAX_REPLY_RECIPIENTS.' participants across selected recipients and explicit mentions.'); + } + }]; + } + /** * @return array */ diff --git a/resources/js/components/commit-thread.tsx b/resources/js/components/commit-thread.tsx index 0f3f36e3a..3c6fc12fb 100644 --- a/resources/js/components/commit-thread.tsx +++ b/resources/js/components/commit-thread.tsx @@ -1334,6 +1334,7 @@ function CommitReply({ ); } +/** Composes a reply with optional recipients inherited from the parent, excluding the signed-in user. */ function ReplyComposer({ parent, onCancel, @@ -1361,6 +1362,9 @@ function ReplyComposer({ .map(([id, handle]) => [id, { id, handle }]), ).values(), ); + const hasRecipientChoice = participants.some( + ({ id }) => id !== parent.user.id, + ); const { body, images, @@ -1510,7 +1514,7 @@ function ReplyComposer({ - {participants.length > 1 && ( + {hasRecipientChoice && ( 1 + hasRecipientChoice ? 'Post your reply' : `Reply to @${parent.user.handle}` } @@ -1553,7 +1557,11 @@ function ReplyComposer({ )}
{!preview.isPreviewing && ( diff --git a/resources/js/components/reply-recipient-picker.tsx b/resources/js/components/reply-recipient-picker.tsx index ecf069914..57845aa9d 100644 --- a/resources/js/components/reply-recipient-picker.tsx +++ b/resources/js/components/reply-recipient-picker.tsx @@ -11,6 +11,7 @@ import { export type ReplyParticipant = { id: string; handle: string }; +/** Lets the author choose additional reply recipients while keeping the parent author selected. */ export function ReplyRecipientPicker({ participants, authorId, @@ -35,12 +36,14 @@ export function ReplyRecipientPicker({ > Replying to{' '} - {selected - .map( - ({ handle }, index) => - `${index > 0 ? (index === selected.length - 1 ? ' and ' : ', ') : ''}@${handle}`, - ) - .join('')} + {selected.length === 0 + ? 'no additional participants' + : selected + .map( + ({ handle }, index) => + `${index > 0 ? (index === selected.length - 1 ? ' and ' : ', ') : ''}@${handle}`, + ) + .join('')} diff --git a/tests/Feature/Controllers/CommitReplyControllerTest.php b/tests/Feature/Controllers/CommitReplyControllerTest.php index d6ab1fa37..633576dd8 100644 --- a/tests/Feature/Controllers/CommitReplyControllerTest.php +++ b/tests/Feature/Controllers/CommitReplyControllerTest.php @@ -8,6 +8,7 @@ use App\Ai\Agents\Tom; use App\Contracts\Services\ToolDecisionProvider; use App\Enums\CommitStatus; +use App\Http\Requests\CreateCommitReplyRequest; use App\Jobs\DetectCommitLanguage; use App\Jobs\ReplyAsTom; use App\Jobs\UnfurlLink; @@ -19,6 +20,35 @@ use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Storage; +it('bounds the combined inherited and explicit reply recipients', function (int $inheritedCount, bool $accepted): void { + Queue::fake(); + config(['app.admins' => ['alice']]); + $author = User::factory()->create(['handle' => 'alice']); + $users = User::factory()->count($inheritedCount)->create(); + $mentions = $users->mapWithKeys(fn (User $user): array => [$user->handle => $user->id])->all(); + $participant = User::factory()->create(['handle' => 'new-participant']); + $parent = Commit::factory()->create(['mentions' => $mentions]); + + $response = $this->actingAs($author)->postJson(route('commit-reply.store', $parent), [ + 'body' => 'Hello @new-participant @'.$users->firstOrFail()->handle, + 'reply_recipients' => array_values($mentions), + ]); + + if ($accepted) { + $response->assertCreated(); + $reply = Commit::query()->findOrFail($response->json('committed.id')); + expect($reply->mentions)->toHaveCount(CreateCommitReplyRequest::MAX_REPLY_RECIPIENTS); + } else { + $response->assertUnprocessable()->assertJsonValidationErrors('reply_recipients'); + expect($parent->replies()->count())->toBe(0); + expect($participant->notifications()->count())->toBe(0); + } +})->with([ + 'at limit with overlapping explicit mention' => [CreateCommitReplyRequest::MAX_REPLY_RECIPIENTS - 1, true], + 'combined recipients over limit' => [CreateCommitReplyRequest::MAX_REPLY_RECIPIENTS, false], + 'inherited recipients over limit' => [CreateCommitReplyRequest::MAX_REPLY_RECIPIENTS + 1, false], +]); + it('only notifies selected conversation recipients and keeps explicit mentions', function (bool $selected, bool $explicit): void { Queue::fake(); config(['app.admins' => ['alice']]); diff --git a/tests/Unit/CommitThread.test.tsx b/tests/Unit/CommitThread.test.tsx index fff3f7fd8..8213fa82b 100644 --- a/tests/Unit/CommitThread.test.tsx +++ b/tests/Unit/CommitThread.test.tsx @@ -7,6 +7,7 @@ import { test } from 'vite-plus/test'; import { ThreadActions, ThreadReplies, + ThreadReplyComposer, useThread, } from '../../resources/js/components/commit-thread'; import { TooltipProvider } from '../../resources/js/components/ui/tooltip'; @@ -91,6 +92,58 @@ const text = (html: string) => .replace(/<[^>]+>/g, ' ') .replace(/\s+/g, ' '); +async function renderReplyComposer(mentions: Record | null) { + const commit = { ...reply('own-commit'), mentions }; + + function Preview() { + const thread = useThread(commit); + return ( + + + + ); + } + + const result = await createInertiaApp({ + page: { + component: 'test', + props: { + auth: { user: commit.user, log: {} }, + errors: {}, + repliesPerPage: 3, + }, + url: '/', + version: null, + clearHistory: false, + encryptHistory: false, + rescuedProps: [], + rememberedState: {}, + flash: {}, + }, + resolve: () => Preview, + render: renderToString, + setup: ({ App, props }) => , + }); + + assert.ok(result); + return result.body; +} + +test('keeps recipient choices available when replying to your own commit with one other participant', async () => { + const html = await renderReplyComposer({ participant: 'other-user' }); + + assert.match(text(html), /Replying to @participant/); + assert.match(html, /placeholder="Post your reply"/); + assert.doesNotMatch(text(html), /Replying to @author/); +}); + +test('keeps the normal placeholder when replying to your own commit without other participants', async () => { + const html = await renderReplyComposer(null); + + assert.doesNotMatch(text(html), /Replying to/); + assert.match(html, /placeholder="Reply to @author"/); +}); + for (const [count, repliesPerPage] of [ [0, 3], [1, 3], From 5766205ea1fcdcce47ba812e19907250e32289b2 Mon Sep 17 00:00:00 2001 From: richardkeep Date: Wed, 28 Oct 2026 07:05:24 +0300 Subject: [PATCH 3/8] Apply CodeRabbit suggestions fixes --- .../js/components/reply-recipient-picker.tsx | 133 +++++++++--------- 1 file changed, 65 insertions(+), 68 deletions(-) diff --git a/resources/js/components/reply-recipient-picker.tsx b/resources/js/components/reply-recipient-picker.tsx index 57845aa9d..d22eaa571 100644 --- a/resources/js/components/reply-recipient-picker.tsx +++ b/resources/js/components/reply-recipient-picker.tsx @@ -7,6 +7,7 @@ import { DialogDescription, DialogHeader, DialogTitle, + DialogTrigger, } from '@/components/ui/dialog'; export type ReplyParticipant = { id: string; handle: string }; @@ -27,73 +28,69 @@ export function ReplyRecipientPicker({ const selected = participants.filter(({ id }) => !excludedIds.includes(id)); return ( - <> - - - - - Replying to - - Choose who to include. The commit author stays - selected. Anyone you mention in your reply will also - be notified. - - -
- {participants.map(({ id, handle }) => ( - - ))} -
- - - - + + + + + + + Replying to + + Choose who to include. The commit author stays selected. + Anyone you mention in your reply will also be notified. + + +
+ {participants.map(({ id, handle }) => ( + + ))} +
+ + + ); } From 238bfc7ce20ba553020c9ceaafa6b261985983c0 Mon Sep 17 00:00:00 2001 From: richardkeep Date: Wed, 28 Oct 2026 07:21:03 +0300 Subject: [PATCH 4/8] Apply CodeRabbit suggestions fixes --- resources/js/components/commit-thread.tsx | 3 + tests/Browser/ReplyComposer.test.ts | 100 ++++++++++++++++++++++ tests/Browser/fixtures/reply-composer.tsx | 52 +++++++++++ tests/tsconfig.json | 2 + 4 files changed, 157 insertions(+) create mode 100644 tests/Browser/ReplyComposer.test.ts create mode 100644 tests/Browser/fixtures/reply-composer.tsx diff --git a/resources/js/components/commit-thread.tsx b/resources/js/components/commit-thread.tsx index 3c6fc12fb..349e88752 100644 --- a/resources/js/components/commit-thread.tsx +++ b/resources/js/components/commit-thread.tsx @@ -1560,6 +1560,9 @@ function ReplyComposer({ message={ errors.body ?? errors.reply_recipients ?? + Object.entries(errors).find(([key]) => + key.startsWith('reply_recipients.'), + )?.[1] ?? errors.link_preview_url } className="px-1" diff --git a/tests/Browser/ReplyComposer.test.ts b/tests/Browser/ReplyComposer.test.ts new file mode 100644 index 000000000..dc20842d9 --- /dev/null +++ b/tests/Browser/ReplyComposer.test.ts @@ -0,0 +1,100 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; +import react from '@vitejs/plugin-react'; +import { chromium } from 'playwright'; +import { createServer } from 'vite'; + +await test( + 'submits only checked recipients and displays indexed recipient errors', + { timeout: 60000 }, + async (context) => { + const server = await createServer({ + configFile: false, + cacheDir: 'node_modules/.vite-reply-composer-tests', + resolve: { + alias: { + '@': fileURLToPath( + new URL('../../resources/js', import.meta.url), + ), + }, + }, + plugins: [ + react(), + { + name: 'reply-composer-fixture', + configureServer(server) { + server.middlewares.use( + '/reply-composer', + async (_request, response) => { + const html = await server.transformIndexHtml( + '/reply-composer', + '
', + ); + response.setHeader('Content-Type', 'text/html'); + response.end(html); + }, + ); + }, + }, + ], + server: { host: '127.0.0.1', port: 0 }, + }); + context.after(() => server.close()); + await server.listen(); + const browser = await chromium.launch(); + try { + const page = await browser.newPage(); + await page.route('**/commits/own-commit/replies', (route) => + route.fulfill({ + status: 422, + json: { + errors: { + 'reply_recipients.0': + 'Recipient is no longer available.', + link_preview_url: + 'The link preview is unavailable.', + }, + }, + }), + ); + const url = server.resolvedUrls?.local[0]; + assert.ok(url); + await page.goto(`${url}reply-composer`); + const trigger = page.getByRole('button', { name: /Replying to/ }); + await trigger.click(); + await page + .getByRole('checkbox', { + name: 'Include @excluded', + exact: true, + }) + .uncheck(); + await page + .getByRole('button', { name: 'Done', exact: true }) + .click(); + await page.waitForFunction( + () => + document.activeElement?.getAttribute('data-slot') === + 'dialog-trigger', + ); + await page.getByRole('textbox').fill('Hello everyone'); + const requestPromise = page.waitForRequest( + (request) => + request.url().endsWith('/commits/own-commit/replies') && + request.method() === 'POST', + ); + await page + .getByRole('button', { name: 'Reply', exact: true }) + .click(); + const request = await requestPromise; + assert.deepEqual(request.postDataJSON().reply_recipients, [ + 'retained-user', + ]); + await page + .getByText('Recipient is no longer available.', { exact: true }) + .waitFor(); + } finally { + await browser.close(); + } + }, +); diff --git a/tests/Browser/fixtures/reply-composer.tsx b/tests/Browser/fixtures/reply-composer.tsx new file mode 100644 index 000000000..475f3f462 --- /dev/null +++ b/tests/Browser/fixtures/reply-composer.tsx @@ -0,0 +1,52 @@ +import { createInertiaApp } from '@inertiajs/react'; +import { createRoot } from 'react-dom/client'; +import { + ThreadReplyComposer, + useThread, +} from '../../../resources/js/components/commit-thread'; +import { TooltipProvider } from '../../../resources/js/components/ui/tooltip'; +import type { Commit } from '../../../resources/js/types'; + +const commit = { + id: 'own-commit', + user_id: 'author', + user: { id: 'author', handle: 'author', name: 'Author' }, + mentions: { + author: 'author', + excluded: 'excluded-user', + retained: 'retained-user', + }, + replies_count: 0, + status: 'approved', +} as unknown as Commit; + +function Composer() { + const thread = useThread(commit); + return ( + + + + ); +} + +await createInertiaApp({ + page: { + component: 'test', + props: { + auth: { user: commit.user, log: {} }, + errors: {}, + repliesPerPage: 3, + }, + url: '/', + version: null, + clearHistory: false, + encryptHistory: false, + rescuedProps: [], + rememberedState: {}, + flash: {}, + }, + resolve: () => Composer, + setup: ({ el, App, props }) => { + if (el) createRoot(el).render(); + }, +}); diff --git a/tests/tsconfig.json b/tests/tsconfig.json index 2c64c4ad6..66a9e1542 100644 --- a/tests/tsconfig.json +++ b/tests/tsconfig.json @@ -7,6 +7,8 @@ "include": [ "Unit/**/*.test.ts", "Unit/**/*.test.tsx", + "Browser/**/*.ts", + "Browser/**/*.tsx", "../resources/js/types/*.d.ts", "draft-images.config.ts" ] From e6c1bcb67c4c0a583c5e15f844cc5f25723608a3 Mon Sep 17 00:00:00 2001 From: richardkeep Date: Wed, 28 Oct 2026 07:31:25 +0300 Subject: [PATCH 5/8] Apply CodeRabbit suggestions fixes --- package.json | 1 + tests/Browser/ReplyComposer.test.ts | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/package.json b/package.json index 330095d10..b6402b593 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "lint": "NODE_OPTIONS='--experimental-strip-types' vp fmt resources/ && NODE_OPTIONS='--experimental-strip-types' vp lint --fix", "test:lint": "NODE_OPTIONS='--experimental-strip-types' vp fmt --check resources/ && NODE_OPTIONS='--experimental-strip-types' vp lint", "test:types": "tsc --noEmit && tsc --noEmit --project tests/tsconfig.json", + "test:browser": "node --experimental-strip-types --test tests/Browser/*.test.ts", "test:unit": "php artisan wayfinder:generate --with-form && node --experimental-strip-types --test tests/Unit/*.test.ts" }, "dependencies": { diff --git a/tests/Browser/ReplyComposer.test.ts b/tests/Browser/ReplyComposer.test.ts index dc20842d9..47e68d501 100644 --- a/tests/Browser/ReplyComposer.test.ts +++ b/tests/Browser/ReplyComposer.test.ts @@ -1,4 +1,5 @@ import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; import { test } from 'node:test'; import { fileURLToPath } from 'node:url'; import react from '@vitejs/plugin-react'; @@ -9,6 +10,10 @@ await test( 'submits only checked recipients and displays indexed recipient errors', { timeout: 60000 }, async (context) => { + execFileSync('php', ['artisan', 'wayfinder:generate', '--with-form'], { + cwd: fileURLToPath(new URL('../../', import.meta.url)), + stdio: 'pipe', + }); const server = await createServer({ configFile: false, cacheDir: 'node_modules/.vite-reply-composer-tests', From d081830e98d718878a201976727d76cbfe3b0f43 Mon Sep 17 00:00:00 2001 From: richardkeep Date: Wed, 28 Oct 2026 07:39:11 +0300 Subject: [PATCH 6/8] Apply CodeRabbit suggestions fixes --- resources/js/components/commit-thread.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/resources/js/components/commit-thread.tsx b/resources/js/components/commit-thread.tsx index 349e88752..23083baf4 100644 --- a/resources/js/components/commit-thread.tsx +++ b/resources/js/components/commit-thread.tsx @@ -680,6 +680,7 @@ export function ThreadReplyComposer({ thread }: { thread: Thread }) { return thread.isReplying && !isDeleted(commit) ? ( Date: Wed, 28 Oct 2026 07:41:39 +0300 Subject: [PATCH 7/8] Apply CodeRabbit suggestions fixes --- package.json | 1 - tests/Browser/ReplyComposer.test.ts | 105 ---------------------------- 2 files changed, 106 deletions(-) delete mode 100644 tests/Browser/ReplyComposer.test.ts diff --git a/package.json b/package.json index b6402b593..330095d10 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,6 @@ "lint": "NODE_OPTIONS='--experimental-strip-types' vp fmt resources/ && NODE_OPTIONS='--experimental-strip-types' vp lint --fix", "test:lint": "NODE_OPTIONS='--experimental-strip-types' vp fmt --check resources/ && NODE_OPTIONS='--experimental-strip-types' vp lint", "test:types": "tsc --noEmit && tsc --noEmit --project tests/tsconfig.json", - "test:browser": "node --experimental-strip-types --test tests/Browser/*.test.ts", "test:unit": "php artisan wayfinder:generate --with-form && node --experimental-strip-types --test tests/Unit/*.test.ts" }, "dependencies": { diff --git a/tests/Browser/ReplyComposer.test.ts b/tests/Browser/ReplyComposer.test.ts deleted file mode 100644 index 47e68d501..000000000 --- a/tests/Browser/ReplyComposer.test.ts +++ /dev/null @@ -1,105 +0,0 @@ -import assert from 'node:assert/strict'; -import { execFileSync } from 'node:child_process'; -import { test } from 'node:test'; -import { fileURLToPath } from 'node:url'; -import react from '@vitejs/plugin-react'; -import { chromium } from 'playwright'; -import { createServer } from 'vite'; - -await test( - 'submits only checked recipients and displays indexed recipient errors', - { timeout: 60000 }, - async (context) => { - execFileSync('php', ['artisan', 'wayfinder:generate', '--with-form'], { - cwd: fileURLToPath(new URL('../../', import.meta.url)), - stdio: 'pipe', - }); - const server = await createServer({ - configFile: false, - cacheDir: 'node_modules/.vite-reply-composer-tests', - resolve: { - alias: { - '@': fileURLToPath( - new URL('../../resources/js', import.meta.url), - ), - }, - }, - plugins: [ - react(), - { - name: 'reply-composer-fixture', - configureServer(server) { - server.middlewares.use( - '/reply-composer', - async (_request, response) => { - const html = await server.transformIndexHtml( - '/reply-composer', - '
', - ); - response.setHeader('Content-Type', 'text/html'); - response.end(html); - }, - ); - }, - }, - ], - server: { host: '127.0.0.1', port: 0 }, - }); - context.after(() => server.close()); - await server.listen(); - const browser = await chromium.launch(); - try { - const page = await browser.newPage(); - await page.route('**/commits/own-commit/replies', (route) => - route.fulfill({ - status: 422, - json: { - errors: { - 'reply_recipients.0': - 'Recipient is no longer available.', - link_preview_url: - 'The link preview is unavailable.', - }, - }, - }), - ); - const url = server.resolvedUrls?.local[0]; - assert.ok(url); - await page.goto(`${url}reply-composer`); - const trigger = page.getByRole('button', { name: /Replying to/ }); - await trigger.click(); - await page - .getByRole('checkbox', { - name: 'Include @excluded', - exact: true, - }) - .uncheck(); - await page - .getByRole('button', { name: 'Done', exact: true }) - .click(); - await page.waitForFunction( - () => - document.activeElement?.getAttribute('data-slot') === - 'dialog-trigger', - ); - await page.getByRole('textbox').fill('Hello everyone'); - const requestPromise = page.waitForRequest( - (request) => - request.url().endsWith('/commits/own-commit/replies') && - request.method() === 'POST', - ); - await page - .getByRole('button', { name: 'Reply', exact: true }) - .click(); - const request = await requestPromise; - assert.deepEqual(request.postDataJSON().reply_recipients, [ - 'retained-user', - ]); - await page - .getByText('Recipient is no longer available.', { exact: true }) - .waitFor(); - } finally { - await browser.close(); - } - }, -); From 218c8f2a75870e85bd9c92c76a7fea08c129dfd1 Mon Sep 17 00:00:00 2001 From: richardkeep Date: Fri, 30 Oct 2026 09:06:14 +0300 Subject: [PATCH 8/8] apply fixes --- KNOWLEDGE_BASE.md | 4 +- app/Models/Commit.php | 4 +- resources/js/components/commit-thread.tsx | 7 ++- resources/js/hooks/use-stash.ts | 13 +++++ tests/Unit/Actions/CreateTomReplyTest.php | 31 ++++++++++++ tests/Unit/DraftStash.test.tsx | 59 +++++++++++++++++++++++ 6 files changed, 111 insertions(+), 7 deletions(-) diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index de152c881..966427259 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -240,7 +240,7 @@ Sources: [composer](resources/js/components/commit-composer.tsx), [issue command ### Stashed drafts -The app stashes unfinished commit and reply text, attached images, and image descriptions locally in your browser so you can return to them. Drafts belong to the signed-in user on that browser; they are not cloud drafts and do not synchronize between devices. Publishing or dropping a draft clears that stash. Browser storage restrictions or clearing site data can remove it; do not use the stash as your only copy of important writing. +The app stashes unfinished commit and reply text, attached images, image descriptions, and reply recipient exclusions locally in your browser so you can return to them. Drafts belong to the signed-in user on that browser; they are not cloud drafts and do not synchronize between devices. Publishing or dropping a draft clears that stash. Browser storage restrictions or clearing site data can remove it; do not use the stash as your only copy of important writing. Sources: [stash hook](resources/js/hooks/use-stash.ts), [image storage](resources/js/lib/image-storage.ts). @@ -747,7 +747,7 @@ Originally, Tom had no particular purpose beyond that nod to MySpace. He now has Tom's lookups start with **OpenAI's Decisions API**, which evaluates simple criteria for each read-only tool using the latest message and the same bounded public thread context. Finding social posts and community reactions uses post search; factual platform questions and notable events, including milestones, downtime and security patches, use the knowledge base; current repository numbers and contribution opportunities use repository insights; feature development and attribution questions use repository history; and supported member counts use community statistics. Sources are complementary: a question about a feature's behavior, evolution and community reception can use the knowledge base, posts and repository history together, without needing to request each search explicitly. Tom compiles the relevant evidence into one answer, distinguishing documented facts, community opinions and development history. An inconclusive search can lead to a follow-up with different keywords, and each new reply has a fresh search allowance. When no tool is needed, such as small talk, ambiguous requests or anything outside his role, Tom simply replies as regular chat. -Tag **@tom** in a public commit or reply, or reply directly to one of his posts without tagging him. Once it is approved, Tom can reply under that commit, at most once, addressing its author rather than the thread's root poster. Replies further down the thread still need to tag him unless their direct parent is a Tom post. Mentions in code do not summon him. His replies use the normal commit moderation and notification pipeline. He does not reply to himself, across a block, in private messages, or to deleted or unapproved commits. +Tag **@tom** in a public commit or reply, or reply directly to one of his posts without tagging him. Once it is approved, Tom can reply under that commit, at most once, addressing its author rather than the thread's root poster. Replies further down the thread still need to tag him unless their direct parent is a Tom post. Mentions in code do not summon him. Selecting him as a reply notification recipient does not summon him either. His replies use the normal commit moderation and notification pipeline. He does not reply to himself, across a block, in private messages, or to deleted or unapproved commits. If the real GitHub owner of `tom` signs in, the bot moves to **@devnull-tom** with the same profile, posts and friendships. Mention **@devnull-tom** to summon him after that; direct replies and mentions captured before the rename still reach the bot. New mentions of the human **@tom** do not summon him. He remains every newcomer's first friend. diff --git a/app/Models/Commit.php b/app/Models/Commit.php index e91a35d4c..fd9d1dff2 100644 --- a/app/Models/Commit.php +++ b/app/Models/Commit.php @@ -179,7 +179,9 @@ public function mentionedNonUsers(): array public function isAddressedTo(User $user): bool { - return in_array($user->id, $this->mentions ?? [], true) || $this->parent?->user_id === $user->id; + $explicitMentions = array_intersect_key($this->mentions ?? [], array_flip(resolve(Mentions::class)->handles($this->body))); + + return in_array($user->id, $explicitMentions, true) || $this->parent?->user_id === $user->id; } /** diff --git a/resources/js/components/commit-thread.tsx b/resources/js/components/commit-thread.tsx index 23083baf4..0095a5b84 100644 --- a/resources/js/components/commit-thread.tsx +++ b/resources/js/components/commit-thread.tsx @@ -1347,9 +1347,6 @@ function ReplyComposer({ }) { const { user } = useUser(); const commitNotFound = useCommitNotFound(); - const [excludedRecipientIds, setExcludedRecipientIds] = useState( - [], - ); const participants = Array.from( new Map( [ @@ -1372,6 +1369,8 @@ function ReplyComposer({ descriptions, isRestored, key: stashKey, + excludedRecipientIds, + updateExcludedRecipientIds, updateBody, updateImages, drop, @@ -1520,7 +1519,7 @@ function ReplyComposer({ participants={participants} authorId={parent.user.id} excludedIds={excludedRecipientIds} - onExcludedIdsChange={setExcludedRecipientIds} + onExcludedIdsChange={updateExcludedRecipientIds} /> )}
diff --git a/resources/js/hooks/use-stash.ts b/resources/js/hooks/use-stash.ts index a1d76c86e..11b9e0240 100644 --- a/resources/js/hooks/use-stash.ts +++ b/resources/js/hooks/use-stash.ts @@ -20,6 +20,8 @@ export type UseStashReturn = { readonly images: File[]; readonly descriptions: string[]; readonly isRestored: boolean; + readonly excludedRecipientIds: string[]; + readonly updateExcludedRecipientIds: (ids: string[]) => void; readonly updateBody: (body: string) => void; readonly updateImages: (images: File[], descriptions?: string[]) => void; readonly drop: () => void; @@ -208,6 +210,14 @@ export function useHasStash(userId: string | null, name: string): boolean { export function useStash(userId: string, name: string): UseStashReturn { const key = keyFor(userId, name); const [body, setBody] = useState(() => read(key)); + const recipientKey = `${key}:excluded-recipients`; + const [excludedRecipientIds, setExcludedRecipientIds] = useState(() => + readStorage(recipientKey).split(',').filter(Boolean), + ); + const updateExcludedRecipientIds = (ids: string[]): void => { + setExcludedRecipientIds(ids); + writeStorage(recipientKey, ids.join(',')); + }; const { draftImages, descriptions, isImagesRestored } = useDraftImages(key); const [isDraftRestored, setIsDraftRestored] = useState( () => body !== '' || draftImages.length > 0, @@ -240,6 +250,7 @@ export function useStash(userId: string, name: string): UseStashReturn { const drop = (): void => { cancelImageRead(key); setBody(''); + updateExcludedRecipientIds([]); setIsDraftRestored(false); discardBody(key); storeImages(key, []); @@ -253,6 +264,8 @@ export function useStash(userId: string, name: string): UseStashReturn { isRestored: (isDraftRestored || isImagesRestored) && (body !== '' || draftImages.length > 0), + excludedRecipientIds, + updateExcludedRecipientIds, updateBody, updateImages, drop, diff --git a/tests/Unit/Actions/CreateTomReplyTest.php b/tests/Unit/Actions/CreateTomReplyTest.php index 8f2e8f73a..f327b42b5 100644 --- a/tests/Unit/Actions/CreateTomReplyTest.php +++ b/tests/Unit/Actions/CreateTomReplyTest.php @@ -292,3 +292,34 @@ Tom::assertNeverPrompted(); expect($commit->replies()->count())->toBe(0); })->with([true, false]); + +it('notifies selected Tom recipients but only invokes him for an explicit body mention', function (bool $approved, string $body, bool $invoked): void { + Queue::fake([ReplyAsTom::class]); + Tom::fake()->preventStrayPrompts(); + $tom = User::factory()->create(['handle' => 'tom', 'github_id' => CreateTomFriendship::GITHUB_ID]); + $parent = Commit::factory()->create(['body' => '@tom hello', 'mentions' => ['tom' => $tom->id]]); + $user = User::factory()->create(); + config(['app.admins' => $approved ? [$user->handle] : []]); + + $commit = resolve(CreateCommit::class)->handle($user, $body, parent: $parent, replyRecipients: ['tom' => $tom->id]); + + if (! $approved) { + Queue::assertNotPushed(ReplyAsTom::class); + resolve(UpdateCommitStatuses::class)->handle([$commit->id], CommitStatus::Approved); + } + + expect($commit->refresh()->mentions)->toBe(['tom' => $tom->id]) + ->and($commit->isAddressedTo($tom))->toBe($invoked); + expect($tom->notifications()->where('commit_id', $commit->id)->exists())->toBeTrue(); + Queue::assertPushed(ReplyAsTom::class, $invoked ? 1 : 0); + + if (! $invoked) { + resolve(CreateTomReply::class)->handle($commit->id); + Tom::assertNeverPrompted(); + expect($commit->replies()->count())->toBe(0); + } +})->with([true, false])->with([ + 'notification only' => ['Thanks for explaining!', false], + 'code mention' => ['This is code: `@tom`', false], + 'explicit mention' => ['@ToM what do you think?', true], +]); diff --git a/tests/Unit/DraftStash.test.tsx b/tests/Unit/DraftStash.test.tsx index 2a332f8ec..39ca48ab3 100644 --- a/tests/Unit/DraftStash.test.tsx +++ b/tests/Unit/DraftStash.test.tsx @@ -342,3 +342,62 @@ test('drafts release image URLs on replacement, drop and logout while keeping re revoke.mockRestore(); } }); + +test('reply recipient exclusions survive closing and reloading a draft and clear on drop or logout', async () => { + vi.resetModules(); + vi.doMock('react', mockReactHooks); + let hooks = await import('../../resources/js/hooks/use-stash'); + vi.stubGlobal('window', { addEventListener() {} }); + vi.useFakeTimers(); + state = []; + const render = (user = 'recipient-author', name = 'reply-parent') => { + cursor = 0; + return hooks.useStash(user, name); + }; + + try { + let draft = render(); + assert.deepEqual(draft.excludedRecipientIds, []); + draft.updateExcludedRecipientIds(['excluded-developer']); + draft.updateBody('My unfinished reply'); + state = []; + draft = render(); + assert.equal(draft.body, 'My unfinished reply'); + assert.deepEqual(draft.excludedRecipientIds, ['excluded-developer']); + vi.advanceTimersByTime(300); + + vi.resetModules(); + hooks = await import('../../resources/js/hooks/use-stash'); + state = []; + draft = render(); + assert.equal(draft.body, 'My unfinished reply'); + assert.deepEqual(draft.excludedRecipientIds, ['excluded-developer']); + state = []; + assert.deepEqual(render('another-author').excludedRecipientIds, []); + state = []; + assert.deepEqual( + render('recipient-author', 'reply-other').excludedRecipientIds, + [], + ); + state = []; + draft = render(); + draft.drop(); + state = []; + draft = render(); + assert.equal(draft.body, ''); + assert.deepEqual(draft.excludedRecipientIds, []); + draft.updateExcludedRecipientIds(['excluded-developer']); + draft.updateImages([image]); + state = []; + draft = render(); + assert.deepEqual(draft.images, [image]); + assert.deepEqual(draft.excludedRecipientIds, ['excluded-developer']); + hooks.clearStashes('recipient-author'); + state = []; + assert.deepEqual(render().excludedRecipientIds, []); + } finally { + hooks.clearStashes('recipient-author'); + vi.useRealTimers(); + vi.unstubAllGlobals(); + } +});