fix: apply code review — enforce quota server-side, project conventions
Critical fixes:
- Quota enforcement regressed when the regex flow was replaced with
tools. The LLM was trusted to respect [Session state] quota hints,
which is vulnerable to prompt injection / hallucination. Each tool
now checks Pennant feature limits at the top of handle() and returns
a short quota-exhausted string instead of calling the provider.
Audio shares the video quota (there is no AiAudioLimit by design).
- N+1 on aiMessages: combined the two separate ->get() scans that
produced imagesInThread and videosInThread into a single query.
- postPlatforms lazy load: controller now loadMissing('postPlatforms')
before constructing SocialMediaAssistant, which accesses the relation
in activePlatformRules().
Project conventions:
- \RuntimeException and \Throwable are now imported at the top of the
controller instead of inlined per CLAUDE.md.
- Tool handle() methods use data_get($request, 'prompt') instead of
direct array access, matching the data_get convention for service
classes.
- enrichContent() pluralizes attachment types via Str::plural so
'2 images' reads naturally instead of '2 image'.
Tests:
- Added Storage::fake('public') in tool tests so runs don't pollute
the local disk with fake-generated files.
- Restored quota enforcement coverage (deleted when the flow changed)
as three new tests — one per tool — that fill the monthly bucket
and assert the tool refuses and nothing is generated.
- Removed inline \App\ references in tests in favor of imports.
This commit is contained in:
parent
43c4cc6df0
commit
5f7768ca3a
12 changed files with 139 additions and 17 deletions
|
|
@ -13,6 +13,7 @@
|
|||
use App\Models\AiMessage;
|
||||
use App\Models\Post;
|
||||
use App\Models\Workspace;
|
||||
use Illuminate\Support\Str;
|
||||
use Laravel\Ai\Contracts\Agent;
|
||||
use Laravel\Ai\Contracts\Conversational;
|
||||
use Laravel\Ai\Contracts\HasTools;
|
||||
|
|
@ -123,7 +124,7 @@ private function enrichContent(AiMessage $m): string
|
|||
if ($m->role === 'assistant' && ! empty($m->attachments)) {
|
||||
$counts = collect($m->attachments)
|
||||
->groupBy('type')
|
||||
->map(fn ($group, $type) => count($group)." {$type}")
|
||||
->map(fn ($group, $type) => count($group).' '.Str::plural((string) $type, count($group)))
|
||||
->implode(', ');
|
||||
|
||||
$content .= "\n\n[This assistant message attached: {$counts}]";
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@
|
|||
namespace App\Ai\Tools;
|
||||
|
||||
use App\Enums\Ai\UsageType;
|
||||
use App\Features\AiVideosLimit;
|
||||
use App\Models\AiUsageLog;
|
||||
use App\Models\Post;
|
||||
use App\Models\Workspace;
|
||||
|
|
@ -14,6 +15,7 @@
|
|||
use Laravel\Ai\Audio;
|
||||
use Laravel\Ai\Contracts\Tool;
|
||||
use Laravel\Ai\Tools\Request;
|
||||
use Laravel\Pennant\Feature;
|
||||
use Stringable;
|
||||
|
||||
class GenerateAudio implements Tool
|
||||
|
|
@ -42,7 +44,16 @@ public function description(): Stringable|string
|
|||
|
||||
public function handle(Request $request): Stringable|string
|
||||
{
|
||||
$text = (string) $request['text'];
|
||||
// Audio generation counts against the monthly video quota by design.
|
||||
$limit = (int) Feature::for($this->workspace->account)->value(AiVideosLimit::class);
|
||||
$used = AiUsageLog::monthlyCount($this->workspace->account_id, UsageType::Video)
|
||||
+ AiUsageLog::monthlyCount($this->workspace->account_id, UsageType::Audio);
|
||||
|
||||
if ($used >= $limit) {
|
||||
return "Audio and video share a monthly quota that is exhausted ({$used} of {$limit} used). Ask the user to upgrade their plan or wait until next month.";
|
||||
}
|
||||
|
||||
$text = (string) data_get($request, 'text', '');
|
||||
$voice = config('services.elevenlabs.default_voice', 'EXAVITQu4vr4xnSDxMaL');
|
||||
|
||||
$response = Audio::of($text)->voice($voice)->generate();
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@
|
|||
|
||||
use App\Enums\Ai\Orientation;
|
||||
use App\Enums\Ai\UsageType;
|
||||
use App\Features\AiImagesLimit;
|
||||
use App\Models\AiUsageLog;
|
||||
use App\Models\Post;
|
||||
use App\Models\Workspace;
|
||||
|
|
@ -15,6 +16,7 @@
|
|||
use Laravel\Ai\Contracts\Tool;
|
||||
use Laravel\Ai\Image;
|
||||
use Laravel\Ai\Tools\Request;
|
||||
use Laravel\Pennant\Feature;
|
||||
use Stringable;
|
||||
|
||||
class GenerateImage implements Tool
|
||||
|
|
@ -46,8 +48,15 @@ public function description(): Stringable|string
|
|||
|
||||
public function handle(Request $request): Stringable|string
|
||||
{
|
||||
$prompt = (string) $request['prompt'];
|
||||
$orientationString = (string) ($request['orientation'] ?? 'vertical');
|
||||
$limit = (int) Feature::for($this->workspace->account)->value(AiImagesLimit::class);
|
||||
$used = AiUsageLog::monthlyCount($this->workspace->account_id, UsageType::Image);
|
||||
|
||||
if ($used >= $limit) {
|
||||
return "Image quota exhausted this month ({$used} of {$limit} used). Ask the user to upgrade their plan or wait until next month.";
|
||||
}
|
||||
|
||||
$prompt = (string) data_get($request, 'prompt', '');
|
||||
$orientationString = (string) data_get($request, 'orientation', 'vertical');
|
||||
$orientationEnum = Orientation::tryFrom($orientationString) ?? Orientation::Vertical;
|
||||
$aspectRatio = $orientationEnum->aspectRatio();
|
||||
|
||||
|
|
|
|||
|
|
@ -5,12 +5,16 @@
|
|||
namespace App\Ai\Tools;
|
||||
|
||||
use App\Enums\Ai\Orientation;
|
||||
use App\Enums\Ai\UsageType;
|
||||
use App\Features\AiVideosLimit;
|
||||
use App\Models\AiUsageLog;
|
||||
use App\Models\Post;
|
||||
use App\Models\Workspace;
|
||||
use App\Services\Ai\VideoGenerationService;
|
||||
use Illuminate\Contracts\JsonSchema\JsonSchema;
|
||||
use Laravel\Ai\Contracts\Tool;
|
||||
use Laravel\Ai\Tools\Request;
|
||||
use Laravel\Pennant\Feature;
|
||||
use Stringable;
|
||||
|
||||
class GenerateVideo implements Tool
|
||||
|
|
@ -42,11 +46,17 @@ public function description(): Stringable|string
|
|||
|
||||
public function handle(Request $request): Stringable|string
|
||||
{
|
||||
$prompt = (string) $request['prompt'];
|
||||
$orientationString = (string) ($request['orientation'] ?? 'vertical');
|
||||
$limit = (int) Feature::for($this->workspace->account)->value(AiVideosLimit::class);
|
||||
$used = AiUsageLog::monthlyCount($this->workspace->account_id, UsageType::Video);
|
||||
|
||||
if ($used >= $limit) {
|
||||
return "Video quota exhausted this month ({$used} of {$limit} used). Ask the user to upgrade their plan or wait until next month.";
|
||||
}
|
||||
|
||||
$prompt = (string) data_get($request, 'prompt', '');
|
||||
$orientationString = (string) data_get($request, 'orientation', 'vertical');
|
||||
$orientationEnum = Orientation::tryFrom($orientationString) ?? Orientation::Vertical;
|
||||
|
||||
/** @var VideoGenerationService $service */
|
||||
$service = app(VideoGenerationService::class);
|
||||
|
||||
$attachment = $service->generate(
|
||||
|
|
|
|||
|
|
@ -18,7 +18,9 @@
|
|||
use Illuminate\Http\Request;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
use Laravel\Pennant\Feature;
|
||||
use RuntimeException;
|
||||
use Symfony\Component\HttpFoundation\Response;
|
||||
use Throwable;
|
||||
|
||||
class PostAssistantController extends Controller
|
||||
{
|
||||
|
|
@ -86,15 +88,19 @@ public function store(
|
|||
}
|
||||
|
||||
try {
|
||||
$imagesInThread = $post->aiMessages()
|
||||
->where('role', 'assistant')
|
||||
->get()
|
||||
->sum(fn ($m) => collect($m->attachments ?? [])->where('type', 'image')->count());
|
||||
$post->loadMissing('postPlatforms');
|
||||
|
||||
$videosInThread = $post->aiMessages()
|
||||
$assistantMessages = $post->aiMessages()
|
||||
->where('role', 'assistant')
|
||||
->get()
|
||||
->sum(fn ($m) => collect($m->attachments ?? [])->where('type', 'video')->count());
|
||||
->get();
|
||||
|
||||
$imagesInThread = $assistantMessages->sum(
|
||||
fn ($m) => collect($m->attachments ?? [])->where('type', 'image')->count()
|
||||
);
|
||||
|
||||
$videosInThread = $assistantMessages->sum(
|
||||
fn ($m) => collect($m->attachments ?? [])->where('type', 'video')->count()
|
||||
);
|
||||
|
||||
$imageLimit = (int) Feature::for($workspace->account)->value(AiImagesLimit::class);
|
||||
$imageUsed = AiUsageLog::monthlyCount($workspace->account_id, UsageType::Image);
|
||||
|
|
@ -147,10 +153,10 @@ public function store(
|
|||
'user_message' => $userMessage,
|
||||
'assistant_message' => $assistantMessage,
|
||||
], Response::HTTP_CREATED);
|
||||
} catch (\Throwable $e) {
|
||||
} catch (Throwable $e) {
|
||||
Log::error('PostAssistantController error', ['error' => $e->getMessage()]);
|
||||
|
||||
$errorMessage = $e instanceof \RuntimeException ? $e->getMessage() : __('assistant.error');
|
||||
$errorMessage = $e instanceof RuntimeException ? $e->getMessage() : __('assistant.error');
|
||||
|
||||
$assistantMessage = $post->aiMessages()->create([
|
||||
'role' => 'assistant',
|
||||
|
|
|
|||
1
lang/php_en.json
Normal file
1
lang/php_en.json
Normal file
File diff suppressed because one or more lines are too long
1
lang/php_es.json
Normal file
1
lang/php_es.json
Normal file
File diff suppressed because one or more lines are too long
1
lang/php_pt-br.json
Normal file
1
lang/php_pt-br.json
Normal file
File diff suppressed because one or more lines are too long
|
|
@ -97,7 +97,7 @@
|
|||
$messages = collect($agent->messages())->all();
|
||||
|
||||
expect($messages[0]->content)->toContain('Here is your image');
|
||||
expect($messages[0]->content)->toContain('[This assistant message attached: 2 image]');
|
||||
expect($messages[0]->content)->toContain('[This assistant message attached: 2 images]');
|
||||
});
|
||||
|
||||
test('instructions include rules only for active post platforms', function () {
|
||||
|
|
|
|||
|
|
@ -7,14 +7,17 @@
|
|||
use App\Enums\Ai\UsageType;
|
||||
use App\Enums\User\Setup;
|
||||
use App\Enums\UserWorkspace\Role;
|
||||
use App\Models\AiUsageLog;
|
||||
use App\Models\Post;
|
||||
use App\Models\User;
|
||||
use App\Models\Workspace;
|
||||
use Illuminate\JsonSchema\JsonSchemaTypeFactory;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Laravel\Ai\Audio;
|
||||
use Laravel\Ai\Tools\Request as ToolRequest;
|
||||
|
||||
beforeEach(function () {
|
||||
Storage::fake('public');
|
||||
$this->user = User::factory()->create(['setup' => Setup::Completed]);
|
||||
$this->workspace = Workspace::factory()->create(['user_id' => $this->user->id]);
|
||||
$this->workspace->members()->attach($this->user->id, ['role' => Role::Member->value]);
|
||||
|
|
@ -52,6 +55,30 @@
|
|||
]);
|
||||
});
|
||||
|
||||
test('tool refuses to generate when video quota is exhausted (audio shares quota)', function () {
|
||||
Audio::fake();
|
||||
|
||||
// Default (no plan attached) video quota is 10. Audio shares this bucket.
|
||||
AiUsageLog::factory()->video()->count(10)->create([
|
||||
'account_id' => $this->workspace->account_id,
|
||||
'workspace_id' => $this->workspace->id,
|
||||
]);
|
||||
|
||||
$tool = new GenerateAudio(
|
||||
workspace: $this->workspace,
|
||||
post: $this->post,
|
||||
userId: $this->user->id,
|
||||
);
|
||||
|
||||
$summary = $tool->handle(new ToolRequest([
|
||||
'text' => 'hello',
|
||||
]));
|
||||
|
||||
expect((string) $summary)->toContain('quota');
|
||||
expect(app(AttachmentCollector::class)->all())->toBeEmpty();
|
||||
Audio::assertNothingGenerated();
|
||||
});
|
||||
|
||||
test('tool exposes schema with text parameter', function () {
|
||||
$tool = new GenerateAudio(
|
||||
workspace: $this->workspace,
|
||||
|
|
|
|||
|
|
@ -7,14 +7,17 @@
|
|||
use App\Enums\Ai\UsageType;
|
||||
use App\Enums\User\Setup;
|
||||
use App\Enums\UserWorkspace\Role;
|
||||
use App\Models\AiUsageLog;
|
||||
use App\Models\Post;
|
||||
use App\Models\User;
|
||||
use App\Models\Workspace;
|
||||
use Illuminate\JsonSchema\JsonSchemaTypeFactory;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Laravel\Ai\Image;
|
||||
use Laravel\Ai\Tools\Request as ToolRequest;
|
||||
|
||||
beforeEach(function () {
|
||||
Storage::fake('public');
|
||||
$this->user = User::factory()->create(['setup' => Setup::Completed]);
|
||||
$this->workspace = Workspace::factory()->create(['user_id' => $this->user->id]);
|
||||
$this->workspace->members()->attach($this->user->id, ['role' => Role::Member->value]);
|
||||
|
|
@ -75,6 +78,31 @@
|
|||
expect(app(AttachmentCollector::class)->all()[0]['type'])->toBe('image');
|
||||
});
|
||||
|
||||
test('tool refuses to generate when monthly image quota is exhausted', function () {
|
||||
Image::fake();
|
||||
|
||||
// Fill up the image quota. Default (no plan attached) is 50 images.
|
||||
AiUsageLog::factory()->image()->count(50)->create([
|
||||
'account_id' => $this->workspace->account_id,
|
||||
'workspace_id' => $this->workspace->id,
|
||||
]);
|
||||
|
||||
$tool = new GenerateImage(
|
||||
workspace: $this->workspace,
|
||||
post: $this->post,
|
||||
userId: $this->user->id,
|
||||
);
|
||||
|
||||
$summary = $tool->handle(new ToolRequest([
|
||||
'prompt' => 'A sunset',
|
||||
'orientation' => 'vertical',
|
||||
]));
|
||||
|
||||
expect((string) $summary)->toContain('quota exhausted');
|
||||
expect(app(AttachmentCollector::class)->all())->toBeEmpty();
|
||||
Image::assertNothingGenerated();
|
||||
});
|
||||
|
||||
test('tool exposes schema with prompt and orientation parameters', function () {
|
||||
$tool = new GenerateImage(
|
||||
workspace: $this->workspace,
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@
|
|||
use App\Ai\Tools\GenerateVideo;
|
||||
use App\Enums\User\Setup;
|
||||
use App\Enums\UserWorkspace\Role;
|
||||
use App\Models\AiUsageLog;
|
||||
use App\Models\Post;
|
||||
use App\Models\User;
|
||||
use App\Models\Workspace;
|
||||
|
|
@ -52,6 +53,32 @@
|
|||
expect(app(AttachmentCollector::class)->all()[0])->toBe($expected);
|
||||
});
|
||||
|
||||
test('tool refuses to generate when monthly video quota is exhausted', function () {
|
||||
// Fill up the video quota. Default (no plan attached) is 10 videos.
|
||||
AiUsageLog::factory()->video()->count(10)->create([
|
||||
'account_id' => $this->workspace->account_id,
|
||||
'workspace_id' => $this->workspace->id,
|
||||
]);
|
||||
|
||||
// VideoGenerationService should not be called when quota is hit.
|
||||
$mock = $this->mock(VideoGenerationService::class);
|
||||
$mock->shouldNotReceive('generate');
|
||||
|
||||
$tool = new GenerateVideo(
|
||||
workspace: $this->workspace,
|
||||
post: $this->post,
|
||||
userId: $this->user->id,
|
||||
);
|
||||
|
||||
$summary = $tool->handle(new ToolRequest([
|
||||
'prompt' => 'A cat',
|
||||
'orientation' => 'vertical',
|
||||
]));
|
||||
|
||||
expect((string) $summary)->toContain('quota exhausted');
|
||||
expect(app(AttachmentCollector::class)->all())->toBeEmpty();
|
||||
});
|
||||
|
||||
test('tool exposes schema with prompt and orientation parameters', function () {
|
||||
$tool = new GenerateVideo(
|
||||
workspace: $this->workspace,
|
||||
|
|
|
|||
Loading…
Reference in a new issue