Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions .claude/commands/login.md

This file was deleted.

2 changes: 1 addition & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ APP_NAME=Laravel
APP_ENV=local
APP_KEY=
APP_DEBUG=true
APP_URL=http://localhost
APP_URL=http://chat.test

APP_LOCALE=en
APP_FALLBACK_LOCALE=en
Expand Down
70 changes: 70 additions & 0 deletions app/Console/Commands/LoginCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php

namespace App\Console\Commands;

use App\Models\User;
use Illuminate\Console\Command;
use Illuminate\Support\Facades\Hash;
use Illuminate\Support\Facades\Process;

class LoginCommand extends Command
{
protected $signature = 'login {--email=dev@test.com : Email for the test user}';

protected $description = 'Ensure a user exists and login via browser';
Comment on lines +10 to +14

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add environment guards to prevent production use.

This command creates users with a known password ('password') and should never run in production. Add an environment check in the handle() method to ensure it only runs in local/testing environments.

Apply this diff to add a production guard:

 public function handle(): int
 {
+    if (! app()->environment(['local', 'testing'])) {
+        $this->error('This command can only be run in local or testing environments.');
+        return self::FAILURE;
+    }
+
     $email = $this->option('email');

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In app/Console/Commands/LoginCommand.php around lines 10 to 14, the command can
create users with a known password and must not run in production; update the
handle() method to check the application environment and abort when running in
production (or non-local/testing) by detecting env via app()->environment() or
config('app.env'), outputting an error message and returning a non-zero exit
code so the command safely refuses to run outside local/testing.


public function handle(): int
{
$email = $this->option('email');

if (! $this->shouldLogin()) {
$this->fixLoginRequirements($email);
}

$url = config('app.url');

$this->info("Logging in as: {$email}");

$result = Process::path(base_path())
->run(['npx', 'playwright', 'test', '--', 'node', 'scripts/login.cjs', $url, $email, 'password']);

if (! $result->successful()) {
// Try direct node execution
$result = Process::path(base_path())
->run(['node', 'scripts/login.cjs', $url, $email, 'password']);
}
Comment on lines +28 to +35

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Simplify Playwright execution - the test runner isn't needed.

The command npx playwright test -- node scripts/login.cjs is attempting to use Playwright's test runner to execute a Node script, which is unnecessary and adds complexity. The script should be run directly with Node.

Apply this diff to simplify:

-        $result = Process::path(base_path())
-            ->run(['npx', 'playwright', 'test', '--', 'node', 'scripts/login.cjs', $url, $email, 'password']);
-
-        if (! $result->successful()) {
-            // Try direct node execution
-            $result = Process::path(base_path())
-                ->run(['node', 'scripts/login.cjs', $url, $email, 'password']);
-        }
+        $result = Process::path(base_path())
+            ->run(['node', 'scripts/login.cjs', $url, $email, 'password']);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$result = Process::path(base_path())
->run(['npx', 'playwright', 'test', '--', 'node', 'scripts/login.cjs', $url, $email, 'password']);
if (! $result->successful()) {
// Try direct node execution
$result = Process::path(base_path())
->run(['node', 'scripts/login.cjs', $url, $email, 'password']);
}
$result = Process::path(base_path())
->run(['node', 'scripts/login.cjs', $url, $email, 'password']);
🤖 Prompt for AI Agents
In app/Console/Commands/LoginCommand.php around lines 28 to 35, the code runs
Playwright's test runner to execute a plain Node script which is unnecessary;
replace the primary Process::run call so it directly executes the script with
Node (e.g., run ['node', 'scripts/login.cjs', $url, $email, 'password']) and
remove the fallback that repeats the same node execution, ensuring the command
only attempts direct node execution once and retains the existing base_path()
context and error handling.


$this->line($result->output());

if ($result->failed()) {
$this->error($result->errorOutput());
return self::FAILURE;
}

return self::SUCCESS;
}

protected function shouldLogin(): bool
{
$email = $this->option('email');

if (! User::where('email', $email)->exists()) {
return false;
}

return true;
}

protected function fixLoginRequirements(string $email): void
{
if (! User::where('email', $email)->exists()) {
$this->info("Creating user: {$email}");

User::create([
'name' => 'Dev User',
'email' => $email,
'password' => Hash::make('password'),
]);
}
}
}
2 changes: 1 addition & 1 deletion eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export default defineConfigWithVueTs(
vue.configs['flat/essential'],
vueTsConfigs.recommended,
{
ignores: ['vendor', 'node_modules', 'public', 'bootstrap/ssr', 'tailwind.config.js', 'resources/js/components/ui/*'],
ignores: ['vendor', 'node_modules', 'public', 'bootstrap/ssr', 'tailwind.config.js', 'resources/js/components/ui/*', 'scripts/*'],
},
{
rules: {
Expand Down
1 change: 0 additions & 1 deletion resources/js/pages/settings/Providers.vue
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
SelectValue,
} from '@/components/ui/select';
import { Checkbox } from '@/components/ui/checkbox';
import InputError from '@/components/InputError.vue';
import { Form } from '@inertiajs/vue3';
import { Trash2, Plus, Eye, EyeOff, Loader2, CheckCircle, XCircle } from 'lucide-vue-next';

Expand Down
20 changes: 20 additions & 0 deletions scripts/login.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
const { chromium } = require('playwright');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix ESLint violation: use ES6 import syntax.

The pipeline flagged this line: CommonJS require() is forbidden. Use ES6 import syntax instead.

Apply this diff to fix the import:

-const { chromium } = require('playwright');
+import { chromium } from 'playwright';

Also update package.json to include "type": "module" if not already present, or rename the file to login.mjs.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 GitHub Actions: linter

[error] 1-1: ESLint: A require() style import is forbidden (@typescript-eslint/no-require-imports) in scripts/login.cjs while running 'eslint . --fix'.

🤖 Prompt for AI Agents
In scripts/login.cjs around line 1, the file uses CommonJS require() which
triggers an ESLint rule forbidding require; replace the CommonJS import with an
ES6 import (import { chromium } from 'playwright') and ensure the runtime
recognizes ESM by either adding "type": "module" to package.json or renaming the
file to login.mjs; update any other require/exports in the file to ESM syntax
and run lint/tests to confirm no other CommonJS usages remain.


const url = process.argv[2] || 'http://chat.test';
const email = process.argv[3] || 'dev@test.com';
const password = process.argv[4] || 'password';

(async () => {
const browser = await chromium.launch({ headless: false });
const page = await browser.newPage();

await page.goto(url + '/login');
await page.fill('input[type="email"], input[name="email"]', email);
await page.fill('input[type="password"], input[name="password"]', password);
await page.click('button[type="submit"]');

await page.waitForURL('**/dashboard**');

const finalUrl = page.url();
console.log('Logged in as ' + email + ' - now on ' + finalUrl);
})();
Comment on lines +7 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Memory leak - browser never closed.

The browser instance is launched but never closed, causing a resource leak. Additionally, there's no error handling if navigation or form interactions fail.

Apply this diff to fix both issues:

 (async () => {
-    const browser = await chromium.launch({ headless: false });
-    const page = await browser.newPage();
-
-    await page.goto(url + '/login');
-    await page.fill('input[type="email"], input[name="email"]', email);
-    await page.fill('input[type="password"], input[name="password"]', password);
-    await page.click('button[type="submit"]');
-
-    await page.waitForURL('**/dashboard**');
-
-    const finalUrl = page.url();
-    console.log('Logged in as ' + email + ' - now on ' + finalUrl);
+    const browser = await chromium.launch({ headless: false });
+    try {
+        const page = await browser.newPage();
+
+        await page.goto(url + '/login', { timeout: 30000 });
+        await page.fill('input[type="email"], input[name="email"]', email);
+        await page.fill('input[type="password"], input[name="password"]', password);
+        await page.click('button[type="submit"]');
+
+        await page.waitForURL('**/dashboard**', { timeout: 30000 });
+
+        const finalUrl = page.url();
+        console.log('Logged in as ' + email + ' - now on ' + finalUrl);
+    } finally {
+        await browser.close();
+    }
 })();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
(async () => {
const browser = await chromium.launch({ headless: false });
const page = await browser.newPage();
await page.goto(url + '/login');
await page.fill('input[type="email"], input[name="email"]', email);
await page.fill('input[type="password"], input[name="password"]', password);
await page.click('button[type="submit"]');
await page.waitForURL('**/dashboard**');
const finalUrl = page.url();
console.log('Logged in as ' + email + ' - now on ' + finalUrl);
})();
(async () => {
const browser = await chromium.launch({ headless: false });
try {
const page = await browser.newPage();
await page.goto(url + '/login', { timeout: 30000 });
await page.fill('input[type="email"], input[name="email"]', email);
await page.fill('input[type="password"], input[name="password"]', password);
await page.click('button[type="submit"]');
await page.waitForURL('**/dashboard**', { timeout: 30000 });
const finalUrl = page.url();
console.log('Logged in as ' + email + ' - now on ' + finalUrl);
} finally {
await browser.close();
}
})();
🤖 Prompt for AI Agents
In scripts/login.cjs around lines 7 to 20, the launched Playwright browser is
never closed and there is no error handling; wrap the async flow in a
try/catch/finally: declare the browser variable outside, assign it when
launching, perform navigation and interactions inside try, log and rethrow or
exit on error in catch, and always call browser.close() in finally (guarding for
undefined) to prevent the memory leak and ensure errors are surfaced.

Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,8 @@
it('prevents toggling model when both credential and model belong to different users', function () {
$user = User::factory()->create();
$other = User::factory()->create();
$otherCredential = UserApiCredential::factory()->for($other)->create();
$anotherCredential = UserApiCredential::factory()->for($other)->create();
$otherCredential = UserApiCredential::factory()->for($other)->openai()->create();
$anotherCredential = UserApiCredential::factory()->for($other)->anthropic()->create();
$model = \App\Models\AiModel::factory()->forCredential($anotherCredential)->create(['enabled' => true]);

$response = $this->actingAs($user)->patch(
Expand Down