diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a1182c..63351fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed +## [0.4.6] - 2026-08-04 + +### Fixed + +- Fixed tracking making prompt rendering fail. Tracking defaulted to on whenever `APP_DEBUG` was false, but its tables are published in a separate opt-in step, so the natural production install threw `no such table: prompt_versions` on the first `Deck::get()`. Caching gave no protection, because the active version is resolved before the cache is consulted. Tracking now defaults to off, and every database interaction degrades to `metadata.json` with a single logged warning rather than throwing. `Deck::track()` never throws at all — it runs after a completed, paid-for AI call. +- Fixed `Deck::activate()` never recording anything in `prompt_versions`. It only ever issued an `UPDATE`, which matched no rows because nothing inserted them, so the table stayed empty and the lookup in `getActiveVersion()` could fail but never succeed. It now upserts, with explicit timestamps, making runtime version switching work as documented. +- Fixed prompt names being interpolated into filesystem paths without validation. A name containing `..` or a directory separator could read files outside the configured prompts path. Names are now validated and `InvalidPromptNameException` is thrown otherwise. +- Fixed the version directory pattern matching far more than intended. Being unanchored and applied to the full path, `/v(\d+)$/` treated directories such as `rev2`, `dev3`, and `archive-v9` as versions 2, 3, and 9 — advertised by `prompt:list --all` and then failing to load. Both `PromptManager` and `make:prompt` now anchor the match against the directory name. +- Fixed `getActiveVersion()` returning the version column unordered and uncast, which picked arbitrarily between multiple active rows and could raise a `TypeError` on drivers that return integers as strings. +- Fixed `make:prompt` creating prompts that could not be loaded back. Kebab-casing passed path separators straight through, so `make:prompt Support/Reply` scaffolded a nested `support/reply` that `PromptManager` refuses to resolve and `prompt:list` shows as a broken `support` entry. The command now validates the name it generated against the same pattern the loader applies, sharing one definition so the two cannot drift apart. +- Fixed `Deck::track()` throwing on an invalid prompt name, contradicting its own documented promise never to throw. It builds no path — the name is only a column value — so the guard protected nothing. +- Fixed the prompt name pattern accepting a trailing newline, since `$` also matches before one. It is now anchored with `\z`. + +### Changed + +- **Tracking now defaults to off.** If you published `config/deck.php` you are unaffected. If you did not and you rely on tracking, set `DECK_TRACKING_ENABLED=true`. See [UPGRADE.md](UPGRADE.md). +- `prompt_versions.user_prompt` is now nullable, so activation can be recorded without prompt content. Existing tracking installs must re-publish and run the migrations. +- Once a version has been activated with tracking enabled, the database takes precedence over `metadata.json`. Editing `active_version` in the file and deploying no longer changes what is served. Activation is environment state; the file is the bootstrap default. + +### Removed + ## [0.4.5] - 2026-08-03 ### Added diff --git a/UPGRADE.md b/UPGRADE.md index e25a65a..c755cfc 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,60 @@ # Upgrade Guide +## Upgrading to `v0.4.6` + +`v0.4.6` is not a breaking release, but two changes need your attention if you use database tracking. + +### Tracking now defaults to off + +Tracking previously defaulted to **on** whenever `APP_DEBUG` was false, which meant a production deploy would enable it before its tables existed — and every prompt load then threw `no such table: prompt_versions`. It now defaults to off: + +```php +'enabled' => env('DECK_TRACKING_ENABLED', false), +``` + +**If you published `config/deck.php`**, your file is unchanged and so is your behaviour. Nothing to do. + +**If you did not publish it and you rely on tracking**, it will switch off silently on upgrade. Turn it back on explicitly: + +```dotenv +DECK_TRACKING_ENABLED=true +``` + +Deck no longer fails when tracking is enabled without its tables. It falls back to `metadata.json` and logs a warning once, so check your logs if tracking data stops appearing. + +### A new migration, if you use tracking + +`activate()` now records the active version in `prompt_versions` — previously it only ever ran an `UPDATE` that matched no rows, so the table was never populated. Inserting requires `user_prompt` to be nullable: + +```bash +php artisan vendor:publish --tag=deck-migrations +php artisan migrate +``` + +Skip this if you do not use tracking. + +### Activation precedence changed in practice + +Because the table is now genuinely populated, the documented rule that the database takes precedence over `metadata.json` starts to have an effect. After the first `Deck::activate()` in an environment, editing `active_version` in `metadata.json` and deploying will no longer change which version is served. Activation is environment state; the file is the bootstrap default. To return a prompt to file-based control, delete its rows from `prompt_versions`. + +### Prompt names are validated + +Names may contain letters, numbers, dots, dashes, and underscores, and may not begin with a dot. Anything else throws `InvalidPromptNameException`. + +**If you organised prompts into subdirectories**, such as `support/reply`, this is a breaking change: those names no longer load. `make:prompt` now refuses to create them too, rather than writing a prompt the package cannot read back. + +Nested prompts never worked properly. `prompt:list` scans only the top level, so `support/reply` was listed as a broken `support` entry at `v0` and never as itself. Flatten the names to restore them: + +```bash +mv resources/prompts/support/reply resources/prompts/support-reply +``` + +Then update any `Deck::get('support/reply')` call to `Deck::get('support-reply')`. + +--- + +## Upgrading from Prompt Deck `v0.3.x` to Deck `v0.4.0` + - [Update Composer](#update-composer) - [Update namespaces](#update-namespaces) - [Update Laravel AI SDK trait imports](#update-laravel-ai-sdk-trait-imports) @@ -8,8 +63,6 @@ - [Clear Laravel caches](#clear-laravel-caches) - [Database notes](#database-notes) -## Upgrading from Prompt Deck `v0.3.x` to Deck `v0.4.0` - Deck `v0.4.0` is a breaking release. The package has moved from `veeqtoh/prompt-deck` to `promptphp/deck`, and the PHP namespace has changed from `Veeqtoh\PromptDeck` to `PromptPHP\Deck`. diff --git a/config/deck.php b/config/deck.php index 40a7d54..6a64136 100644 --- a/config/deck.php +++ b/config/deck.php @@ -57,9 +57,19 @@ | If enabled, prompt versions and executions will be logged to the database, | enabling performance tracking and audit trails. | + | Tracking is off by default because it requires the package migrations to + | have been published and run. Prompt rendering never depends on it: if the + | tables are missing or the database is unreachable, Deck falls back to + | metadata.json and logs a warning rather than failing. + | + | To enable it: + | 1. php artisan vendor:publish --tag=deck-migrations + | 2. php artisan migrate + | 3. DECK_TRACKING_ENABLED=true + | */ 'tracking' => [ - 'enabled' => env('DECK_TRACKING_ENABLED', env('APP_DEBUG', false) ? false : true), + 'enabled' => env('DECK_TRACKING_ENABLED', false), 'connection' => env('DECK_DB_CONNECTION'), // null for default ], diff --git a/docs/advanced/tracking.mdx b/docs/advanced/tracking.mdx index 6b50aba..a2781c8 100644 --- a/docs/advanced/tracking.mdx +++ b/docs/advanced/tracking.mdx @@ -14,36 +14,36 @@ Deck by PromptPHP includes an optional database tracking system that logs prompt - **Cost analysis** — Monitor API spending per prompt, model, and provider. - **User feedback** — Attach ratings and comments to individual executions. -Tracking is entirely optional. Deck works fully without it. +Tracking is entirely optional and **off by default**. Deck works fully without it. + + + Prompt rendering never depends on tracking. If you enable it without running the migrations, or the database becomes unreachable, Deck falls back to `metadata.json` and logs a warning once rather than failing. `Deck::track()` never throws under any circumstances — it runs after a completed, paid-for AI call. + ## Setup -### Enable tracking +Publish and run the migrations **before** enabling the flag, or tracking has no tables to write to. -Set the tracking configuration in `config/deck.php`: +### Publish and run the migrations -```php -'tracking' => [ - 'enabled' => env('DECK_TRACKING_ENABLED', true), - 'connection' => env('DECK_DB_CONNECTION'), -], +```bash +php artisan vendor:publish --tag=deck-migrations +php artisan migrate ``` -Or via environment variable: +### Enable tracking ```dotenv DECK_TRACKING_ENABLED=true ``` -By default, tracking is **disabled** when `APP_DEBUG=true` and **enabled** in production. - -### Publish migrations - -Publish and run the migrations to create the required tables: +Or set it directly in `config/deck.php`: -```bash -php artisan vendor:publish --tag=deck-migrations -php artisan migrate +```php +'tracking' => [ + 'enabled' => env('DECK_TRACKING_ENABLED', false), + 'connection' => env('DECK_DB_CONNECTION'), +], ``` ### Database connection @@ -54,7 +54,7 @@ By default, tracking uses your application's default database connection. To sto DECK_DB_CONNECTION=analytics ``` -The connection name must match a connection defined in `config/database.php`. +The connection name must match a connection defined in `config/database.php`. If it does not, Deck logs a warning and falls back to `metadata.json` rather than failing. ## Database schema @@ -381,9 +381,19 @@ $dailyCost = PromptExecution::where('prompt_name', 'order-summary') ## Version management via database -When tracking is enabled, version activation is managed through the `prompt_versions` table instead of `metadata.json` files. This provides a centralised, queryable record of version history. +When tracking is enabled, `Deck::activate()` and `prompt:activate` record the active version in the `prompt_versions` table **in addition to** `metadata.json`, giving you a centralised, queryable record of version history. -The `Deck::activate()` method and `prompt:activate` command automatically use the appropriate storage (database or file) based on your tracking configuration. +### Activation precedence + +Once a version has been activated with tracking enabled, **the database wins**. `metadata.json` is consulted only when the table holds no record for that prompt. + +The practical consequence: editing `active_version` in `metadata.json`, committing it, and deploying **will not change which version is served**. Activation is environment state — it is how you promote a version in staging without touching production, and how you roll back without a deploy. The file is the bootstrap default, used until the first activation in that environment. + +To go back to file-based control for a prompt, delete its rows from `prompt_versions`. + + + The table only ever holds versions that have been **activated**. It is not a mirror of your prompt library — a version you have created but never activated has no row. The `system_prompt` and `user_prompt` columns are nullable and are written as `NULL`: Deck is file-based, and content lives on disk. + ```php // Activate programmatically diff --git a/docs/changelog.mdx b/docs/changelog.mdx index acae31a..0af3718 100644 --- a/docs/changelog.mdx +++ b/docs/changelog.mdx @@ -6,6 +6,35 @@ rss: true Deck follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Subscribe to the [RSS feed](https://deck.promptphp.com/changelog/rss.xml) to be notified of new releases, or browse the [full history on GitHub](https://github.com/promptphp/deck/blob/0.x/CHANGELOG.md). + + + + Tracking now defaults to **off**. If you published `config/deck.php` you are unaffected. If you did not and you rely on tracking, set `DECK_TRACKING_ENABLED=true`. If you use tracking, re-publish and run the migrations. + + If you organised prompts into subdirectories such as `support/reply`, those names no longer load — flatten them to `support-reply`. Nested prompts never worked properly: `prompt:list` scans only the top level and showed them as a broken entry. See the [upgrade guide](https://github.com/promptphp/deck/blob/0.x/UPGRADE.md). + + +**Fixed** + +- **Tracking no longer breaks prompt rendering.** Tracking defaulted to on whenever `APP_DEBUG` was false, but its tables are published in a separate opt-in step — so the natural production install (`composer require`, publish config, deploy) threw `no such table: prompt_versions` on the very first `Deck::get()`. Caching gave no protection, because the active version is resolved before the cache is consulted. + + Tracking now defaults to off, and every database interaction degrades to `metadata.json` with a single logged warning instead of throwing. `Deck::track()` never throws under any circumstances: it runs after a completed, paid-for AI call, and no analytics failure is worth discarding that response. + +- **`Deck::activate()` now records the version it activated.** It only ever issued an `UPDATE`, which matched no rows because nothing ever inserted them — so `prompt_versions` stayed empty and the lookup that read it could fail but never succeed. Activation is now an upsert, making runtime version switching work as documented. +- **Prompt names are validated.** A name was interpolated straight into a filesystem path, so one containing `..` or a directory separator could read files outside your prompts directory. Names now throw `InvalidPromptNameException` unless they contain only letters, numbers, dots, dashes, and underscores. +- **Version directories are matched precisely.** The pattern was unanchored and applied to the full path, so `rev2`, `dev3`, and `archive-v9` registered as versions 2, 3, and 9 — advertised by `prompt:list --all`, then failing to load. +- **`make:prompt` can no longer create a prompt that will not load.** Kebab-casing passed path separators through, so `make:prompt Support/Reply` scaffolded a nested `support/reply` that the manager refuses to resolve and `prompt:list` displays as a broken `support` entry. The generator and the loader now share one definition of a valid name, so they cannot drift apart. +- **`Deck::track()` no longer throws on an invalid name**, which contradicted its own documented promise. It builds no path — the name is only a column value — so the guard protected nothing. +- `getActiveVersion()` no longer picks arbitrarily between multiple active rows, and casts the version it reads. +- The name pattern no longer accepts a trailing newline, since `$` matches before one. + +**Changed** + +- `prompt_versions.user_prompt` is nullable, so activation can be recorded without prompt content. Deck is file-based; content lives on disk. +- Once a version has been activated with tracking enabled, the database takes precedence over `metadata.json`. Editing `active_version` in the file and deploying no longer changes what is served — activation is environment state, the file is the bootstrap default. See [Tracking — Activation precedence](/advanced/tracking#activation-precedence). + + + **Added** diff --git a/docs/core/prompts.mdx b/docs/core/prompts.mdx index c0971fa..a3bfb04 100644 --- a/docs/core/prompts.mdx +++ b/docs/core/prompts.mdx @@ -39,6 +39,18 @@ $prompt = Deck::get('order-summary', 2); The `get` method returns a `PromptTemplate` instance. If no version is specified, the [active version](#version-resolution-order) is resolved automatically. +### Prompt names + +A prompt name becomes a directory path, so names are validated before they are resolved. They may contain letters, numbers, dots, dashes, and underscores, and may not begin with a dot. Anything else — most importantly a name containing `/`, `\`, or `..` — throws `InvalidPromptNameException`. + +```php +Deck::get('order-summary'); // fine +Deck::get('order_summary'); // fine +Deck::get('../../.env'); // InvalidPromptNameException +``` + +This matters if a prompt name ever comes from user input, where an unvalidated name could otherwise read files outside your prompts directory. + ### Dependency injection You can also inject the `PromptManager` directly via Laravel's service container: diff --git a/docs/getting-started/configuration.mdx b/docs/getting-started/configuration.mdx index 65118ac..a2e99b8 100644 --- a/docs/getting-started/configuration.mdx +++ b/docs/getting-started/configuration.mdx @@ -148,27 +148,41 @@ The `tracking` section controls whether prompt versions and executions are logge ```php 'tracking' => [ - 'enabled' => env('DECK_TRACKING_ENABLED', env('APP_DEBUG', false) ? false : true), + 'enabled' => env('DECK_TRACKING_ENABLED', false), 'connection' => env('DECK_DB_CONNECTION'), ], ``` ### Enabling / disabling -```php -'enabled' => env('DECK_TRACKING_ENABLED', env('APP_DEBUG', false) ? false : true), -``` - -Like caching, tracking is **disabled** in debug mode and **enabled** in production by default. When enabled: - -- **Version activation** is stored in the `prompt_versions` database table (instead of `metadata.json`). +Tracking is **off by default**, because it needs database tables that are published in a separate step. Turning it on takes three: + + + + ```bash + php artisan vendor:publish --tag=deck-migrations + ``` + + + ```bash + php artisan migrate + ``` + + + ```dotenv + DECK_TRACKING_ENABLED=true + ``` + + + +When enabled: + +- **Version activation** is recorded in the `prompt_versions` table *as well as* `metadata.json`, and the table takes precedence. See [Tracking — Activation precedence](/advanced/tracking#activation-precedence). - **Execution tracking** via `Deck::track()` inserts records into the `prompt_executions` table. - - You must publish and run the migrations before enabling tracking. See - [Installation — Publishing - migrations](/getting-started/installation#publishing-migrations). - + + Prompt rendering never depends on tracking. If the tables are missing or the database is unreachable, Deck falls back to `metadata.json` and logs a warning once, rather than failing. `Deck::track()` never throws at all — it runs after a completed AI call, and no analytics failure is worth discarding that response. + ### Database connection diff --git a/src/Concerns/ValidatesPromptNames.php b/src/Concerns/ValidatesPromptNames.php new file mode 100644 index 0000000..00d1eec --- /dev/null +++ b/src/Concerns/ValidatesPromptNames.php @@ -0,0 +1,35 @@ +toKebabCase($rawName); + $name = $this->toKebabCase($rawName); + + // toKebabCase() passes separators straight through, so 'Support/Reply' + // became 'support/reply' — scaffolded into a nested directory that + // PromptManager then refuses to resolve, and that prompt:list cannot + // display. Fail here rather than writing a prompt nothing can read. + if (! $this->isValidPromptName($name)) { + $this->error("Prompt name [{$name}] is not valid. Names may contain letters, numbers, dots, dashes, and underscores, and may not begin with a dot."); + + return Command::FAILURE; + } + $basePath = config('deck.path'); // Resolve description. @@ -209,7 +222,10 @@ protected function detectLatestVersion(string $promptPath): int $latest = 0; foreach ($this->files->directories($promptPath) as $dir) { - if (preg_match('/v(\d+)$/', $dir, $matches)) { + // Anchored against the directory name alone, matching + // PromptManager::versions(). The two must agree on what counts as + // a version, or the command scaffolds over an existing one. + if (preg_match('/^v(\d+)$/', basename($dir), $matches)) { $latest = max($latest, (int) $matches[1]); } } diff --git a/src/Database/migrations/2025_02_28_0000001_create_prompt_versions.php b/src/Database/migrations/2025_02_28_0000001_create_prompt_versions.php index acd57ed..966d515 100644 --- a/src/Database/migrations/2025_02_28_0000001_create_prompt_versions.php +++ b/src/Database/migrations/2025_02_28_0000001_create_prompt_versions.php @@ -15,7 +15,7 @@ public function up() $table->string('name'); $table->unsignedInteger('version'); $table->text('system_prompt')->nullable(); - $table->text('user_prompt'); + $table->text('user_prompt')->nullable(); $table->json('metadata')->nullable(); $table->boolean('is_active')->default(false); $table->timestamps(); diff --git a/src/Database/migrations/2026_08_04_000001_make_prompt_versions_content_nullable.php b/src/Database/migrations/2026_08_04_000001_make_prompt_versions_content_nullable.php new file mode 100644 index 0000000..3485478 --- /dev/null +++ b/src/Database/migrations/2026_08_04_000001_make_prompt_versions_content_nullable.php @@ -0,0 +1,34 @@ +text('user_prompt')->nullable()->change(); + }); + } + + public function down(): void + { + // Rows written by activate() carry no content, so they must go before + // the column can be NOT NULL again. + Schema::table('prompt_versions', function (Blueprint $table) { + $table->text('user_prompt')->nullable(false)->default('')->change(); + }); + } +}; diff --git a/src/Exceptions/InvalidPromptNameException.php b/src/Exceptions/InvalidPromptNameException.php new file mode 100644 index 0000000..7e81a08 --- /dev/null +++ b/src/Exceptions/InvalidPromptNameException.php @@ -0,0 +1,24 @@ +files = new Filesystem; @@ -50,6 +71,8 @@ public function __construct(string $basePath, string $extension, Cache $cache, C */ public function get(string $name, string|int|null $version = null): PromptTemplate { + $this->assertValidName($name); + if ($version === null) { $version = $this->getActiveVersion($name); } else { @@ -94,6 +117,8 @@ public function get(string $name, string|int|null $version = null): PromptTempla */ public function active(string $name): PromptTemplate { + $this->assertValidName($name); + return $this->get($name, $this->getActiveVersion($name)); } @@ -102,6 +127,8 @@ public function active(string $name): PromptTemplate */ public function versions(string $name): array { + $this->assertValidName($name); + $promptPath = "{$this->basePath}/{$name}"; if (! $this->files->isDirectory($promptPath)) { @@ -110,11 +137,14 @@ public function versions(string $name): array $versions = []; - // Scan for version directories (v1, v2, etc.) or version files. + // Scan for version directories (v1, v2, etc.). $items = $this->files->directories($promptPath); foreach ($items as $dir) { - if (preg_match('/v(\d+)$/', $dir, $matches)) { + // Anchored against the directory name alone: an unanchored match + // on the full path treats 'rev2', 'dev3' and 'archive-v9' as + // versions 2, 3 and 9. + if (preg_match('/^v(\d+)$/', basename($dir), $matches)) { $version = (int) $matches[1]; $versions[] = [ 'version' => $version, @@ -146,29 +176,44 @@ public function versions(string $name): array */ public function activate(string $name, string|int $version): bool { + $this->assertValidName($name); + $version = $this->parseVersion((string) $version) ?? throw InvalidVersionException::unparseable($name, $version); $this->ensureVersionExists($name, $version); - if ($this->trackingConfig['enabled'] ?? false) { - $connection = DB::connection( - $this->trackingConfig['connection'] ?? config('database.default') - ); - - // Update database. - $connection->transaction(function () use ($connection, $name, $version): void { - $connection - ->table('prompt_versions') - ->where('name', $name) - ->update(['is_active' => false]); - - $connection - ->table('prompt_versions') - ->where('name', $name) - ->where('version', $version) - ->update(['is_active' => true]); - }); + if ($connection = $this->trackingConnection()) { + try { + $connection->transaction(function () use ($connection, $name, $version): void { + // Deactivate every other version of this prompt. + $connection + ->table('prompt_versions') + ->where('name', $name) + ->update(['is_active' => false]); + + // Record this one, inserting it if it has not been activated + // before. Timestamps are passed explicitly: this is a query + // builder call, so Eloquent does not maintain them. + $connection + ->table('prompt_versions') + ->updateOrInsert( + ['name' => $name, 'version' => $version], + ['is_active' => true, 'updated_at' => now(), 'created_at' => now()], + ); + }); + } catch (QueryException $e) { + // Only a missing table is a degradable condition. A deadlock, a + // constraint violation from concurrent activations, or a full + // disk must surface: swallowing them would write metadata.json, + // return true, and leave the database — which getActiveVersion() + // prefers — still pointing at the previous version. + if (! $this->trackingTableMissing($connection, 'prompt_versions')) { + throw $e; + } + + $this->markTrackingUnavailable($e); + } } // Fallback: store in a JSON file in the prompt directory. @@ -185,45 +230,69 @@ public function activate(string $name, string|int $version): bool /** * Track an execution for performance monitoring. + * + * Never throws. This runs after a completed — and paid for — AI call, so + * no analytics failure is worth destroying the response that triggered it. */ public function track(string $promptName, int $version, array $data): void { - if (! ($this->trackingConfig['enabled'] ?? false)) { + // Deliberately not validated: track() builds no path, the name is only + // a column value, so a guard here would protect nothing while breaking + // the promise above. + if (! ($connection = $this->trackingConnection())) { return; } - DB::connection($this->trackingConfig['connection'] ?? config('database.default')) - ->table('prompt_executions') - ->insert([ - 'prompt_name' => $promptName, - 'prompt_version' => $version, - 'input' => json_encode($data['input'] ?? null), - 'output' => $data['output'] ?? null, - 'tokens' => $data['tokens'] ?? null, - 'latency_ms' => $data['latency'] ?? null, - 'cost' => $data['cost'] ?? null, - 'model' => $data['model'] ?? null, - 'provider' => $data['provider'] ?? null, - 'feedback' => isset($data['feedback']) ? json_encode($data['feedback']) : null, - 'created_at' => now(), - ]); + try { + $connection + ->table('prompt_executions') + ->insert([ + 'prompt_name' => $promptName, + 'prompt_version' => $version, + 'input' => json_encode($data['input'] ?? null), + 'output' => $data['output'] ?? null, + 'tokens' => $data['tokens'] ?? null, + 'latency_ms' => $data['latency'] ?? null, + 'cost' => $data['cost'] ?? null, + 'model' => $data['model'] ?? null, + 'provider' => $data['provider'] ?? null, + 'feedback' => isset($data['feedback']) ? json_encode($data['feedback']) : null, + 'created_at' => now(), + ]); + } catch (Throwable $e) { + // Deliberately broad: a missing table, an unreachable host, or a + // json_encode failure on non-UTF-8 input must all be swallowed. + $this->markTrackingUnavailable($e); + } } /** * Get the active version number for a prompt. + * + * The database wins when it holds a record, so a version activated at + * runtime takes precedence over the active_version committed alongside + * the prompt files. */ protected function getActiveVersion(string $name): int { // Check database first if tracking enabled. - if ($this->trackingConfig['enabled'] ?? false) { - $record = DB::connection($this->trackingConfig['connection'] ?? config('database.default')) - ->table('prompt_versions') - ->where('name', $name) - ->where('is_active', true) - ->first(); - - if ($record) { - return $record->version; + if ($connection = $this->trackingConnection()) { + try { + $record = $connection + ->table('prompt_versions') + ->where('name', $name) + ->where('is_active', true) + ->orderByDesc('version') + ->first(); + + if ($record) { + return (int) $record->version; + } + } catch (QueryException $e) { + // Rendering must survive any database problem, not only a + // missing table: serving the version on disk beats not serving + // at all. Latched, so this is attempted once per instance. + $this->markTrackingUnavailable($e); } } @@ -244,6 +313,78 @@ protected function getActiveVersion(string $name): int return max(array_column($versions, 'version')); } + /** + * Resolve the connection tracking should write to, or null when tracking + * is disabled or the database has already been found unusable. + * + * Catching Throwable is deliberate: an undefined DECK_DB_CONNECTION throws + * InvalidArgumentException from the connection resolver rather than a + * QueryException, and is a likely misconfiguration. + */ + protected function trackingConnection(): ?Connection + { + if (! ($this->trackingConfig['enabled'] ?? false) || $this->trackingUnavailable) { + return null; + } + + try { + return DB::connection($this->trackingConfig['connection'] ?? config('database.default')); + } catch (Throwable $e) { + $this->markTrackingUnavailable($e); + + return null; + } + } + + /** + * Record that tracking is unusable, warning once for this instance. + * + * The latch keeps a broken database from being retried on every prompt + * load, and doubles as the guard against flooding the log. + */ + protected function markTrackingUnavailable(Throwable $e): void + { + if ($this->trackingUnavailable) { + return; + } + + $this->trackingUnavailable = true; + + Log::warning( + 'Deck tracking is enabled but the tracking database is unavailable. ' + .'Prompt rendering has fallen back to metadata.json. Publish and run ' + .'the Deck migrations, or set DECK_TRACKING_ENABLED=false. ' + .$e->getMessage() + ); + } + + /** + * Determine whether a tracking table is genuinely absent, as opposed to + * present but erroring for some other reason. + */ + protected function trackingTableMissing(Connection $connection, string $table): bool + { + try { + return ! Schema::connection($connection->getName())->hasTable($table); + } catch (Throwable) { + // If the schema cannot be inspected either, the database is not in + // a usable state — treat it as absent so callers degrade. + return true; + } + } + + /** + * Ensure a prompt name is safe to interpolate into a filesystem path. + * + * @throws InvalidPromptNameException + */ + protected function assertValidName(string $name): void + { + if (! $this->isValidPromptName($name)) { + throw InvalidPromptNameException::named($name); + } + } + /** * Load prompt data from filesystem for a given name and version. * diff --git a/tests/Feature/Commands/MakePromptCommandTest.php b/tests/Feature/Commands/MakePromptCommandTest.php index c713911..21a78f6 100644 --- a/tests/Feature/Commands/MakePromptCommandTest.php +++ b/tests/Feature/Commands/MakePromptCommandTest.php @@ -754,3 +754,37 @@ ->doesntExpectOutputToContain('is still the active version') ->assertSuccessful(); }); + +// ===================================================================== +// The generator may only create names the loader can resolve +// ===================================================================== + +test('make:prompt refuses a name containing a path separator', function () { + // kebab-casing passed '/' straight through, scaffolding a nested prompt + // that PromptManager refuses to load and prompt:list cannot display. + $this->artisan('make:prompt', ['name' => 'Support/Reply']) + ->expectsOutputToContain('is not valid') + ->assertFailed(); + + expect(is_dir("{$this->tempDir}/support"))->toBeFalse(); +}); + +test('make:prompt refuses a traversing name', function () { + $this->artisan('make:prompt', ['name' => '../escape']) + ->expectsOutputToContain('is not valid') + ->assertFailed(); +}); + +test('every name make:prompt accepts can be loaded back', function () { + // Distinct after kebab-casing, so none collides with an earlier one and + // triggers the interactive "already exists" prompt. + foreach (['order-summary', 'BillingReminder', 'ops_alert', 'weekly.digest'] as $input) { + $this->artisan('make:prompt', ['name' => $input])->assertSuccessful(); + } + + // Whatever the generator wrote, the manager must resolve. + foreach (glob("{$this->tempDir}/*", GLOB_ONLYDIR) as $dir) { + $name = basename($dir); + expect(app(PromptManager::class)->get($name)->version())->toBe(1); + } +}); diff --git a/tests/Feature/DeckServiceProviderTest.php b/tests/Feature/DeckServiceProviderTest.php index c99f5d3..7bb1006 100644 --- a/tests/Feature/DeckServiceProviderTest.php +++ b/tests/Feature/DeckServiceProviderTest.php @@ -83,8 +83,14 @@ $source = array_key_first($publishes); + // Asserted by name rather than by count, so adding a migration does not + // break the guard. What matters is that the directory really resolves and + // really holds the table definitions. + $migrations = array_map('basename', glob(rtrim($source, '/').'/*.php')); + expect(is_dir($source))->toBeTrue() - ->and(glob(rtrim($source, '/').'/*.php'))->toHaveCount(2); + ->and($migrations)->toContain('2025_02_28_0000001_create_prompt_versions.php') + ->and($migrations)->toContain('2025_02_28_0000001_create_prompt_executions.php'); }); test('stubs are not included in default provider publishing', function () { @@ -97,3 +103,17 @@ expect($stubPaths)->toBeEmpty(); }); + +test('tracking defaults to off in the package config', function () { + // Tracking needs migrations that are published in a separate, opt-in step, + // so it must not default on: prompt rendering would depend on tables that + // may not exist. + // + // Asserted against the source rather than the evaluated value, because + // phpunit.xml pins DECK_TRACKING_ENABLED for the whole suite, which would + // mask the default either way. + $source = file_get_contents(__DIR__.'/../../config/deck.php'); + + expect($source)->toContain("'enabled' => env('DECK_TRACKING_ENABLED', false)") + ->and($source)->not->toContain("env('DECK_TRACKING_ENABLED', env('APP_DEBUG'"); +}); diff --git a/tests/Feature/MigrationTest.php b/tests/Feature/MigrationTest.php index 1d0bfe9..9be634d 100644 --- a/tests/Feature/MigrationTest.php +++ b/tests/Feature/MigrationTest.php @@ -557,3 +557,21 @@ function rollbackMigrations(): void expect($execution->provider)->toBeIn(['openai', 'anthropic']); }); + +test('prompt_versions user_prompt is nullable so activate() can record a version', function () { + runMigrations(); + + DB::connection('testing')->table('prompt_versions')->insert([ + 'name' => 'content-free', + 'version' => 1, + 'is_active' => true, + 'created_at' => now(), + 'updated_at' => now(), + ]); + + $record = DB::connection('testing')->table('prompt_versions') + ->where('name', 'content-free')->first(); + + expect($record->user_prompt)->toBeNull() + ->and($record->system_prompt)->toBeNull(); +}); diff --git a/tests/Unit/PromptManagerTest.php b/tests/Unit/PromptManagerTest.php index 139e330..1fbfc4d 100644 --- a/tests/Unit/PromptManagerTest.php +++ b/tests/Unit/PromptManagerTest.php @@ -3,6 +3,8 @@ declare(strict_types=1); use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; +use PromptPHP\Deck\Exceptions\InvalidPromptNameException; use PromptPHP\Deck\Exceptions\InvalidVersionException; use PromptPHP\Deck\Exceptions\PromptNotFoundException; use PromptPHP\Deck\PromptManager; @@ -641,3 +643,255 @@ function freshManager(?array $configOverrides = []): PromptManager ->and($manager->activate('parity', $input))->toBeTrue(); } }); + +// ===================================================================== +// Tracking enabled without its tables — must degrade, never throw +// ===================================================================== + +test('get() falls back to metadata.json when the tracking tables are absent', function () { + // Deliberately does NOT call setUpTrackingTables(). + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('no-tables', 1, 'sys v1', null, null, ['active_version' => 1]); + $this->createPromptFixture('no-tables', 2, 'sys v2'); + + $prompt = freshManager()->get('no-tables'); + + expect($prompt->version())->toBe(1) + ->and($prompt->system())->toBe('sys v1'); +}); + +test('track() is a silent no-op when the tracking table is absent', function () { + config()->set('deck.tracking.enabled', true); + + freshManager()->track('anything', 1, ['output' => 'hello']); +})->throwsNoExceptions(); + +test('activate() still writes metadata.json when the tracking table is absent', function () { + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('no-tables-activate', 1, 'sys'); + $this->createPromptFixture('no-tables-activate', 2, 'sys'); + + expect(freshManager()->activate('no-tables-activate', 2))->toBeTrue(); + + $meta = json_decode(file_get_contents("{$this->tempDir}/no-tables-activate/metadata.json"), true); + expect($meta['active_version'])->toBe(2); +}); + +test('an undefined tracking connection degrades instead of throwing', function () { + // Throws InvalidArgumentException from the connection resolver rather + // than a QueryException, so a QueryException-only guard would miss it. + config()->set('deck.tracking.enabled', true); + config()->set('deck.tracking.connection', 'does-not-exist'); + + $this->createPromptFixture('bad-connection', 1, 'sys', null, null, ['active_version' => 1]); + + expect(freshManager()->get('bad-connection')->version())->toBe(1); +}); + +test('an unusable tracking database is attempted once, not on every load', function () { + config()->set('deck.tracking.enabled', true); + Log::spy(); + + $this->createPromptFixture('latched', 1, 'sys', null, null, ['active_version' => 1]); + + $manager = freshManager(); + $manager->get('latched'); + $manager->get('latched'); + $manager->get('latched'); + + Log::shouldHaveReceived('warning')->once(); +}); + +// ===================================================================== +// Tracking enabled with its tables — activation is recorded +// ===================================================================== + +test('activate() inserts a prompt_versions row with timestamps', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('db-activate', 1, 'sys v1'); + $this->createPromptFixture('db-activate', 2, 'sys v2'); + + freshManager()->activate('db-activate', 2); + + $row = DB::connection('testing')->table('prompt_versions') + ->where('name', 'db-activate')->where('version', 2)->first(); + + expect($row)->not->toBeNull() + ->and((bool) $row->is_active)->toBeTrue() + ->and($row->created_at)->not->toBeNull() + ->and($row->updated_at)->not->toBeNull(); +}); + +test('getActiveVersion() reads the version back from the database', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('db-roundtrip', 1, 'sys v1'); + $this->createPromptFixture('db-roundtrip', 2, 'sys v2'); + + $manager = freshManager(); + $manager->activate('db-roundtrip', 2); + + expect($manager->get('db-roundtrip')->version())->toBe(2) + ->and($manager->get('db-roundtrip')->system())->toBe('sys v2'); +}); + +test('activate() deactivates sibling versions of the same prompt', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('siblings', 1, 'sys'); + $this->createPromptFixture('siblings', 2, 'sys'); + + $manager = freshManager(); + $manager->activate('siblings', 1); + $manager->activate('siblings', 2); + + $active = DB::connection('testing')->table('prompt_versions') + ->where('name', 'siblings')->where('is_active', true)->pluck('version')->all(); + + expect($active)->toBe([2]); +}); + +test('activating the same version twice updates rather than duplicating', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('twice', 1, 'sys'); + + $manager = freshManager(); + $manager->activate('twice', 1); + $manager->activate('twice', 1); + + $count = DB::connection('testing')->table('prompt_versions')->where('name', 'twice')->count(); + + expect($count)->toBe(1); +}); + +test('the database wins over metadata.json once a version has been activated', function () { + // Activation is environment state; the file is the bootstrap default. + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('precedence', 1, 'sys v1'); + $this->createPromptFixture('precedence', 2, 'sys v2'); + + $manager = freshManager(); + $manager->activate('precedence', 2); + + // Simulate someone editing active_version in git and deploying. + file_put_contents( + "{$this->tempDir}/precedence/metadata.json", + json_encode(['active_version' => 1]), + ); + + expect($manager->get('precedence')->version())->toBe(2); +}); + +test('get() with an explicit version performs no database query', function () { + $this->setUpTrackingTables(); + config()->set('deck.tracking.enabled', true); + + $this->createPromptFixture('hot-path', 2, 'sys v2'); + + $manager = freshManager(); + + DB::connection('testing')->enableQueryLog(); + $manager->get('hot-path', 2); + + expect(DB::connection('testing')->getQueryLog())->toBe([]); +}); + +// ===================================================================== +// Prompt name validation +// ===================================================================== + +test('get() rejects a traversing prompt name', function () { + freshManager()->get('../elsewhere', 1); +})->throws(InvalidPromptNameException::class); + +test('traversal cannot read a prompt outside the configured path', function () { + $outside = dirname($this->tempDir).'/deck-outside-'.uniqid(); + mkdir($outside.'/v1', 0755, true); + file_put_contents($outside.'/v1/system.md', 'SECRET'); + + try { + freshManager()->get('../'.basename($outside), 1); + $leaked = true; + } catch (InvalidPromptNameException) { + $leaked = false; + } finally { + unlink($outside.'/v1/system.md'); + rmdir($outside.'/v1'); + rmdir($outside); + } + + expect($leaked)->toBeFalse(); +}); + +test('name validation rejects separators and leading dots but allows ordinary names', function () { + $manager = freshManager(); + + foreach (['../secrets', '..', 'a/b', 'a\\b', '.hidden', ''] as $bad) { + expect(fn () => $manager->versions($bad))->toThrow(InvalidPromptNameException::class); + } + + // Kebab, snake, and dotted names must keep working. + foreach (['order-summary', 'order_summary', 'order.summary.v2', 'OrderSummary'] as $good) { + $this->createPromptFixture($good, 1, 'sys'); + expect($manager->versions($good))->toHaveCount(1); + } +}); + +test('activate() rejects invalid names, track() does not', function () { + // activate() resolves a path and so must validate. track() only writes a + // column value, and is documented never to throw. + $manager = freshManager(); + + expect(fn () => $manager->activate('../evil', 1))->toThrow(InvalidPromptNameException::class) + ->and(fn () => $manager->track('../evil', 1, []))->not->toThrow(InvalidPromptNameException::class); +}); + +// ===================================================================== +// Version directory detection +// ===================================================================== + +test('versions() ignores directories that merely end in a version-like suffix', function () { + $this->createPromptFixture('phantom', 1, 'sys'); + $this->createPromptFixture('phantom', 10, 'sys'); + + foreach (['rev2', 'dev3', 'archive-v9', 'drafts', 'v'] as $decoy) { + mkdir("{$this->tempDir}/phantom/{$decoy}", 0755, true); + } + + $found = array_column(freshManager()->versions('phantom'), 'version'); + + expect($found)->toBe([1, 10]); +}); + +// ===================================================================== +// track() must never throw — it runs after a paid-for AI call +// ===================================================================== + +test('track() does not throw on a name it would otherwise reject', function () { + // track() builds no path; the name is only a column value. Validating it + // here would guard nothing while breaking the never-throws promise. + config()->set('deck.tracking.enabled', true); + + freshManager()->track('../not/a/real/name', 1, ['output' => 'hi']); +})->throwsNoExceptions(); + +test('track() does not throw when the name is valid but the table is absent', function () { + config()->set('deck.tracking.enabled', true); + + freshManager()->track('order-summary', 1, ['output' => 'hi']); +})->throwsNoExceptions(); + +test('prompt names are rejected when padded with a trailing newline', function () { + // $ matches before a final newline, so the pattern is \z-anchored. + freshManager()->versions("order-summary\n"); +})->throws(InvalidPromptNameException::class);