[#2302] Added support for prompts schema to make the installer AI-friendly.#2306
Merged
AlexSkrypnyk merged 1 commit intomainfrom Feb 19, 2026
Merged
[#2302] Added support for prompts schema to make the installer AI-friendly.#2306AlexSkrypnyk merged 1 commit intomainfrom
AlexSkrypnyk merged 1 commit intomainfrom
Conversation
WalkthroughAdds machine-readable schema, config validation, and agent instruction outputs to the installer; extends handler API with envName/type/dependsOn; refactors PromptManager to a dispatcher; adds SchemaGenerator, ConfigValidator, Normalizer; updates many handlers/tests to use per-handler env names; and adds comprehensive unit/functional tests. Changes
Sequence Diagram(s)sequenceDiagram
actor Agent
participant CLI as InstallCommand
participant PM as PromptManager
participant SM as SchemaGenerator
participant CV as ConfigValidator
participant H as Handlers
Agent->>CLI: --schema
CLI->>PM: getHandlers()
PM-->>CLI: handlers[]
CLI->>SM: generate(handlers)
SM->>H: read handler metadata
SM-->>CLI: schema JSON
CLI-->>Agent: schema JSON
Agent->>CLI: --validate --config=...
CLI->>PM: getHandlers()
PM-->>CLI: handlers[]
CLI->>CV: validate(config, handlers)
CV->>H: normalize keys & check dependencies
CV->>H: validate types, apply defaults
CV-->>CLI: {valid, errors, warnings, resolved}
CLI-->>Agent: validation JSON
Agent->>CLI: --agent-help
CLI-->>Agent: agent instruction text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/installer/src/Command/InstallCommand.php:
- Around line 319-326: The current code casts file_get_contents($config_option)
to string which hides read failures; update the logic around
$config_json/$user_config so that when is_file($config_option) is true you call
file_get_contents($config_option) and explicitly check its return value for ===
FALSE, write a clear error via $output->writeln (e.g. "Failed to read config
file: $config_option") and return Command::FAILURE, otherwise proceed to
json_decode($config_json, true) and keep the existing is_array($user_config)
check; reference variables/functions: $config_option, $config_json,
file_get_contents, $user_config, json_decode.
In @.vortex/installer/src/Prompts/Handlers/AbstractHandler.php:
- Around line 75-99: The type() method currently calls $this->options([]) and
$this->default([]) which can produce incorrect inference when those methods
depend on runtime responses (e.g., DatabaseDownloadSource::options() filtered by
HostingProvider); change type() to accept an optional $responses parameter
(e.g., type(array $responses = []): PromptType) and pass $responses through to
$this->options($responses) and $this->default($responses), then update all
callers to provide the real responses when available; if callers cannot provide
responses, add a docblock on type() noting that calling it with an empty array
yields only a best-effort schema inference for generation time.
In @.vortex/installer/src/Prompts/PromptManager.php:
- Around line 576-582: The prompt() method directly indexes $this->handlers with
handler_class::id() which can cause an uncaught PHP error; change prompt() to
validate the handler key the same way args() does (use
array_key_exists($handler_class::id(), $this->handlers) or call
$this->args($handler_class, ...) to obtain/verify the handler) and throw or
propagate the same RuntimeException when the handler is missing so error
handling is consistent between prompt() and args().
In @.vortex/installer/src/Schema/ConfigValidator.php:
- Around line 99-104: The validation currently calls $handler->options([]) which
ignores context-dependent option filtering; update validateType and any callers
(look for $this->validateType and $handler->options([]) between the block around
199-239) to pass the current normalized/resolved responses array into options()
instead of an empty array (e.g., $handler->options($this->responses) or the
method that returns normalized responses), so conditional options are evaluated
using actual answers during validation.
In @.vortex/installer/src/Schema/SchemaGenerator.php:
- Around line 35-45: The calls to static methods are inconsistent: replace
instance-style calls with static-call syntax for the static methods on the
handler; specifically change $handler->envName() to $handler::envName() and
$handler->description([]) to $handler::description([]) so they match the
existing static call to $handler::id() (leave non-static methods like options(),
default(), isRequired(), dependsOn() as-is).
In @.vortex/installer/src/Utils/Normalizer.php:
- Around line 7-14: The class docblock is incorrect (copy-pasted "Installer
configuration"); update the docblock for the final class Normalizer to
accurately describe its purpose (e.g., what normalization it performs and its
role in the installer), replacing the mistaken "Installer configuration" text
while keeping relevant tags like `@package` DrevOps\VortexInstaller and any
author/license tags intact so the Normalizer class is properly documented.
In @.vortex/installer/tests/Functional/Command/SchemaValidateCommandTest.php:
- Around line 46-57: In testSchemaOutputIsValidJson(), replace the blind
json_decode call with one that throws on parse errors by using
JSON_THROW_ON_ERROR so malformed output fails with a clear exception; update the
json_decode invocation that consumes applicationGetOutput() (and any related
error handling in testSchemaOutputIsValidJson) to pass JSON_THROW_ON_ERROR and,
if needed, wrap in a try/catch to convert the JsonException into a test failure
with the output included for diagnostics.
In
@.vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php:
- Around line 35-40: Remove the unnecessary AiCodeInstructions::envName()
setting from the container_registry test case: inside the test closure that sets
DatabaseDownloadSource::envName() to DatabaseDownloadSource::CONTAINER_REGISTRY
and DatabaseImage::envName() to 'the_empire/star_wars:latest', delete the
Env::put(AiCodeInstructions::envName(), Env::TRUE) call since AiCodeInstructions
is not used by DatabaseDownloadSource or DatabaseImage and the s3 test already
verifies functionality without it.
In @.vortex/installer/tests/Unit/Prompts/PromptTypeTest.php:
- Around line 84-91: The test method name is inconsistent with what it verifies:
rename the method PromptTypeTest::testFromInvalidString to
PromptTypeTest::testTryFromInvalidString so it matches the `@covers` ::tryFrom
annotation and the assertions that call PromptType::tryFrom('...'); update any
references to the old method name (e.g., test suite filters) if present.
In @.vortex/installer/tests/Unit/Schema/ConfigValidatorTest.php:
- Around line 106-117: Replace the hardcoded 'services' string in
testInvalidMultiselectValue with the handler's ID constant (use Services::id())
and add the corresponding import for
DrevOps\VortexInstaller\Prompts\Handlers\Services so the assertion uses the
canonical handler identifier; update the assertion that builds/compares error
prompts to use Services::id() instead of the literal 'services'.
- Around line 122-134: The test testMissingRequiredField only checks that
$result has a 'resolved' key; update it to assert actual validation behavior by
calling $this->validator->validate($config, $this->handlers) and then: 1) assert
that $result contains an 'errors' key and that errors include any required
fields that lack defaults (e.g. assert array key 'Name' or whichever field has
no default is present in $result['errors']); 2) assert that fields with defaults
(e.g. 'Name' and 'HostingProvider' per test comments) appear in
$result['resolved'] with their expected default values; and 3) if applicable
assert that no unexpected errors exist (e.g. empty error list for fields with
defaults) so the test verifies both error and resolution behavior of validate.
In @.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php:
- Around line 30-54: The static caches static::$schema and static::$handlers
used by getSchema() can leak state between tests; modify the test class to
initialize these caches in a setUpBeforeClass() (creating Config, PromptManager,
SchemaGenerator and populating static::$handlers/static::$schema) and clear them
in tearDownAfterClass() (setting static::$schema and static::$handlers back to
NULL) so each test run starts from a known state, or remove the statics and make
getSchema() return a fresh schema/handlers per call to ensure test isolation.
Comment on lines
+319
to
+326
| $config_json = is_file($config_option) ? (string) file_get_contents($config_option) : $config_option; | ||
| $user_config = json_decode($config_json, TRUE); | ||
|
|
||
| if (!is_array($user_config)) { | ||
| $output->writeln('Invalid JSON in --config.'); | ||
|
|
||
| return Command::FAILURE; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Handle file_get_contents failure explicitly.
Line 319 casts file_get_contents() result to string, which converts FALSE (on failure) to an empty string. This empty string then fails json_decode() silently. Consider checking for file read failure explicitly to provide a more helpful error message.
♻️ Proposed improvement
- $config_json = is_file($config_option) ? (string) file_get_contents($config_option) : $config_option;
+ if (is_file($config_option)) {
+ $config_json = file_get_contents($config_option);
+ if ($config_json === FALSE) {
+ $output->writeln(sprintf('Unable to read config file: %s', $config_option));
+ return Command::FAILURE;
+ }
+ }
+ else {
+ $config_json = $config_option;
+ }
$user_config = json_decode($config_json, TRUE);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/src/Command/InstallCommand.php around lines 319 - 326, The
current code casts file_get_contents($config_option) to string which hides read
failures; update the logic around $config_json/$user_config so that when
is_file($config_option) is true you call file_get_contents($config_option) and
explicitly check its return value for === FALSE, write a clear error via
$output->writeln (e.g. "Failed to read config file: $config_option") and return
Command::FAILURE, otherwise proceed to json_decode($config_json, true) and keep
the existing is_array($user_config) check; reference variables/functions:
$config_option, $config_json, file_get_contents, $user_config, json_decode.
Comment on lines
+75
to
+99
| public function type(): PromptType { | ||
| $options = $this->options([]); | ||
|
|
||
| if (is_array($options)) { | ||
| if (array_is_list($options)) { | ||
| return PromptType::Suggest; | ||
| } | ||
|
|
||
| $default = $this->default([]); | ||
|
|
||
| if (is_array($default)) { | ||
| return PromptType::MultiSelect; | ||
| } | ||
|
|
||
| return PromptType::Select; | ||
| } | ||
|
|
||
| $default = $this->default([]); | ||
|
|
||
| if (is_bool($default)) { | ||
| return PromptType::Confirm; | ||
| } | ||
|
|
||
| return PromptType::Text; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type inference may produce inconsistent results when responses affect options/defaults.
The type() method calls $this->options([]) and $this->default([]) with empty arrays. However, some handlers may return different options or defaults depending on responses (e.g., DatabaseDownloadSource::options() filters options based on HostingProvider). This means the inferred type at schema generation time may differ from runtime behavior.
Consider documenting this limitation or ensuring the type inference is only used for schema generation where an empty responses context is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/src/Prompts/Handlers/AbstractHandler.php around lines 75 -
99, The type() method currently calls $this->options([]) and $this->default([])
which can produce incorrect inference when those methods depend on runtime
responses (e.g., DatabaseDownloadSource::options() filtered by HostingProvider);
change type() to accept an optional $responses parameter (e.g., type(array
$responses = []): PromptType) and pass $responses through to
$this->options($responses) and $this->default($responses), then update all
callers to provide the real responses when available; if callers cannot provide
responses, add a docblock on type() noting that calling it with an empty array
yields only a best-effort schema inference for generation time.
Comment on lines
+576
to
+582
| private function prompt(string $handler_class, array $responses = []): mixed { | ||
| $handler = $this->handlers[$handler_class::id()]; | ||
| $fn = '\\Laravel\\Prompts\\' . $handler->type()->promptFunction(); | ||
|
|
||
| // @phpstan-ignore callable.nonCallable | ||
| return $fn(...$this->args($handler_class, NULL, $responses)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent error handling between prompt() and args().
Line 577 accesses $this->handlers without validation, while args() (lines 603-605) validates with array_key_exists and throws a proper RuntimeException. If an invalid handler class is passed, this will throw a PHP error before the cleaner exception in args() can be reached.
♻️ Proposed fix for consistent validation
private function prompt(string $handler_class, array $responses = []): mixed {
+ $id = $handler_class::id();
+
+ if (!array_key_exists($id, $this->handlers)) {
+ throw new \RuntimeException(sprintf('Handler for "%s" not found.', $id));
+ }
+
- $handler = $this->handlers[$handler_class::id()];
+ $handler = $this->handlers[$id];
$fn = '\\Laravel\\Prompts\\' . $handler->type()->promptFunction();
// `@phpstan-ignore` callable.nonCallable
return $fn(...$this->args($handler_class, NULL, $responses));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/src/Prompts/PromptManager.php around lines 576 - 582, The
prompt() method directly indexes $this->handlers with handler_class::id() which
can cause an uncaught PHP error; change prompt() to validate the handler key the
same way args() does (use array_key_exists($handler_class::id(),
$this->handlers) or call $this->args($handler_class, ...) to obtain/verify the
handler) and throw or propagate the same RuntimeException when the handler is
missing so error handling is consistent between prompt() and args().
Comment on lines
+99
to
+104
| // Validate value based on type. | ||
| $type_error = $this->validateType($handler, $value); | ||
| if ($type_error !== NULL) { | ||
| $errors[] = [ | ||
| 'prompt' => $id, | ||
| 'message' => $type_error, |
There was a problem hiding this comment.
Validate conditional options using actual responses.
Line 201 uses $handler->options([]), so context-dependent options (e.g., filtered by HostingProvider) are ignored, which can allow invalid values to pass validation. Pass normalized/resolved responses into options() during validation.
🛠️ Proposed fix
- $type_error = $this->validateType($handler, $value);
+ $context = $normalized + $resolved;
+ $type_error = $this->validateType($handler, $value, $context);- * `@param` mixed $value
- * The value to validate.
+ * `@param` mixed $value
+ * The value to validate.
+ * `@param` array<string, mixed> $responses
+ * Normalized/resolved responses used to evaluate conditional options.- protected function validateType(HandlerInterface $handler, mixed $value): ?string {
+ protected function validateType(HandlerInterface $handler, mixed $value, array $responses): ?string {
$type = $handler->type();
- $options = $handler->options([]);
+ $options = $handler->options($responses);Also applies to: 199-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/src/Schema/ConfigValidator.php around lines 99 - 104, The
validation currently calls $handler->options([]) which ignores context-dependent
option filtering; update validateType and any callers (look for
$this->validateType and $handler->options([]) between the block around 199-239)
to pass the current normalized/resolved responses array into options() instead
of an empty array (e.g., $handler->options($this->responses) or the method that
returns normalized responses), so conditional options are evaluated using actual
answers during validation.
Comment on lines
35
to
40
| 'db download source, container_registry' => [ | ||
| static::cw(function (): void { | ||
| Env::put(PromptManager::makeEnvName(DatabaseDownloadSource::id()), DatabaseDownloadSource::CONTAINER_REGISTRY); | ||
| Env::put(PromptManager::makeEnvName(DatabaseImage::id()), 'the_empire/star_wars:latest'); | ||
| Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), Env::TRUE); | ||
| Env::put(DatabaseDownloadSource::envName(), DatabaseDownloadSource::CONTAINER_REGISTRY); | ||
| Env::put(DatabaseImage::envName(), 'the_empire/star_wars:latest'); | ||
| Env::put(AiCodeInstructions::envName(), Env::TRUE); | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if AiCodeInstructions is actually processed/required when DatabaseDownloadSource is CONTAINER_REGISTRY
rg -n "AiCodeInstructions|CONTAINER_REGISTRY" .vortex/installer/src/Prompts/Handlers/DatabaseDownloadSource.php
echo "---"
# Check if the DatabaseImage handler has any dependency on AiCodeInstructions
rg -n "AiCodeInstructions" .vortex/installer/src/Prompts/Handlers/DatabaseImage.php
echo "---"
# Also check the test file itself to understand what the test is expecting
rg -n "container_registry" .vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php -A 15Repository: drevops/vortex
Length of output: 788
🏁 Script executed:
#!/bin/bash
# Get the full test method for container_registry case to see assertions/expectations
rg -n "container_registry" .vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php -B 2 -A 30 | head -50Repository: drevops/vortex
Length of output: 629
🏁 Script executed:
#!/bin/bash
# Get full test class to understand the test structure and assertions
wc -l .vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php
# Read the file to see test method and assertions
cat -n .vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.phpRepository: drevops/vortex
Length of output: 2234
🏁 Script executed:
#!/bin/bash
# Find and read the AbstractHandlerProcessTestCase to understand what the test does
fd "AbstractHandlerProcessTestCase" .vortex/installer/tests --type f
# Also check if there are other test files in the Handlers directory
ls -la .vortex/installer/tests/Functional/Handlers/Repository: drevops/vortex
Length of output: 2371
🏁 Script executed:
#!/bin/bash
# Read the AbstractHandlerProcessTestCase
cat -n .vortex/installer/tests/Functional/Handlers/AbstractHandlerProcessTestCase.phpRepository: drevops/vortex
Length of output: 4548
Remove unnecessary AiCodeInstructions::envName() from container_registry test case.
The AiCodeInstructions environment variable is not used by either DatabaseDownloadSource or DatabaseImage handlers and is only set in this one test case. The s3 test case demonstrates that the container_registry source functions without it. Remove line 39.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
@.vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php
around lines 35 - 40, Remove the unnecessary AiCodeInstructions::envName()
setting from the container_registry test case: inside the test closure that sets
DatabaseDownloadSource::envName() to DatabaseDownloadSource::CONTAINER_REGISTRY
and DatabaseImage::envName() to 'the_empire/star_wars:latest', delete the
Env::put(AiCodeInstructions::envName(), Env::TRUE) call since AiCodeInstructions
is not used by DatabaseDownloadSource or DatabaseImage and the s3 test already
verifies functionality without it.
Comment on lines
84
to
91
| /** | ||
| * @covers ::tryFrom | ||
| */ | ||
| public function testFromInvalidString(): void { | ||
| $this->assertNull(PromptType::tryFrom('invalid')); | ||
| $this->assertNull(PromptType::tryFrom('')); | ||
| $this->assertNull(PromptType::tryFrom('TEXT')); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor naming inconsistency.
The method testFromInvalidString tests PromptType::tryFrom() behavior, but the name suggests it tests from(). Consider renaming to testTryFromInvalidString to align with the @covers ::tryFrom annotation.
✏️ Suggested rename
/**
* `@covers` ::tryFrom
*/
- public function testFromInvalidString(): void {
+ public function testTryFromInvalidString(): void {
$this->assertNull(PromptType::tryFrom('invalid'));
$this->assertNull(PromptType::tryFrom(''));
$this->assertNull(PromptType::tryFrom('TEXT'));
}📝 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
| /** | |
| * @covers ::tryFrom | |
| */ | |
| public function testFromInvalidString(): void { | |
| $this->assertNull(PromptType::tryFrom('invalid')); | |
| $this->assertNull(PromptType::tryFrom('')); | |
| $this->assertNull(PromptType::tryFrom('TEXT')); | |
| } | |
| /** | |
| * `@covers` ::tryFrom | |
| */ | |
| public function testTryFromInvalidString(): void { | |
| $this->assertNull(PromptType::tryFrom('invalid')); | |
| $this->assertNull(PromptType::tryFrom('')); | |
| $this->assertNull(PromptType::tryFrom('TEXT')); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/tests/Unit/Prompts/PromptTypeTest.php around lines 84 -
91, The test method name is inconsistent with what it verifies: rename the
method PromptTypeTest::testFromInvalidString to
PromptTypeTest::testTryFromInvalidString so it matches the `@covers` ::tryFrom
annotation and the assertions that call PromptType::tryFrom('...'); update any
references to the old method name (e.g., test suite filters) if present.
Comment on lines
+106
to
+117
| public function testInvalidMultiselectValue(): void { | ||
| $config = [ | ||
| 'services' => ['nonexistent_service'], | ||
| ]; | ||
|
|
||
| $result = $this->validator->validate($config, $this->handlers); | ||
|
|
||
| $this->assertFalse($result['valid']); | ||
|
|
||
| $error_prompts = array_column($result['errors'], 'prompt'); | ||
| $this->assertContains('services', $error_prompts); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use handler constant instead of hardcoded string.
Line 108 uses the hardcoded string 'services' while other tests consistently use handler class constants like HostingProvider::id(). This is inconsistent and could become fragile if the handler ID changes.
♻️ Proposed fix
Add the import:
use DrevOps\VortexInstaller\Prompts\Handlers\Services;Then update the test:
public function testInvalidMultiselectValue(): void {
$config = [
- 'services' => ['nonexistent_service'],
+ Services::id() => ['nonexistent_service'],
];
$result = $this->validator->validate($config, $this->handlers);
$this->assertFalse($result['valid']);
$error_prompts = array_column($result['errors'], 'prompt');
- $this->assertContains('services', $error_prompts);
+ $this->assertContains(Services::id(), $error_prompts);
}📝 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
| public function testInvalidMultiselectValue(): void { | |
| $config = [ | |
| 'services' => ['nonexistent_service'], | |
| ]; | |
| $result = $this->validator->validate($config, $this->handlers); | |
| $this->assertFalse($result['valid']); | |
| $error_prompts = array_column($result['errors'], 'prompt'); | |
| $this->assertContains('services', $error_prompts); | |
| } | |
| public function testInvalidMultiselectValue(): void { | |
| $config = [ | |
| Services::id() => ['nonexistent_service'], | |
| ]; | |
| $result = $this->validator->validate($config, $this->handlers); | |
| $this->assertFalse($result['valid']); | |
| $error_prompts = array_column($result['errors'], 'prompt'); | |
| $this->assertContains(Services::id(), $error_prompts); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/tests/Unit/Schema/ConfigValidatorTest.php around lines 106
- 117, Replace the hardcoded 'services' string in testInvalidMultiselectValue
with the handler's ID constant (use Services::id()) and add the corresponding
import for DrevOps\VortexInstaller\Prompts\Handlers\Services so the assertion
uses the canonical handler identifier; update the assertion that builds/compares
error prompts to use Services::id() instead of the literal 'services'.
Comment on lines
+122
to
+134
| public function testMissingRequiredField(): void { | ||
| // Name is required. Provide empty config. | ||
| $config = []; | ||
|
|
||
| $result = $this->validator->validate($config, $this->handlers); | ||
|
|
||
| // Should have errors for required fields that don't have defaults. | ||
| // Name is required but has a default via default(), so it may resolve. | ||
| // HostingProvider is required and has a default, so it resolves. | ||
| // Check that resolved includes defaults. | ||
| $this->assertArrayHasKey('resolved', $result); | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Strengthen assertion to verify actual validation behavior.
The test only verifies that the resolved key exists, which doesn't meaningfully test the required field behavior. Consider asserting specific outcomes, such as verifying that required fields without defaults produce errors or that fields with defaults appear in resolved.
♻️ Proposed improvement
public function testMissingRequiredField(): void {
- // Name is required. Provide empty config.
$config = [];
$result = $this->validator->validate($config, $this->handlers);
- // Should have errors for required fields that don't have defaults.
- // Name is required but has a default via default(), so it may resolve.
- // HostingProvider is required and has a default, so it resolves.
- // Check that resolved includes defaults.
$this->assertArrayHasKey('resolved', $result);
+
+ // Verify fields with defaults are resolved.
+ $this->assertArrayHasKey(HostingProvider::id(), $result['resolved']);
+ $this->assertSame(HostingProvider::NONE, $result['resolved'][HostingProvider::id()]);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/tests/Unit/Schema/ConfigValidatorTest.php around lines 122
- 134, The test testMissingRequiredField only checks that $result has a
'resolved' key; update it to assert actual validation behavior by calling
$this->validator->validate($config, $this->handlers) and then: 1) assert that
$result contains an 'errors' key and that errors include any required fields
that lack defaults (e.g. assert array key 'Name' or whichever field has no
default is present in $result['errors']); 2) assert that fields with defaults
(e.g. 'Name' and 'HostingProvider' per test comments) appear in
$result['resolved'] with their expected default values; and 3) if applicable
assert that no unexpected errors exist (e.g. empty error list for fields with
defaults) so the test verifies both error and resolution behavior of validate.
Comment on lines
+30
to
+54
| /** | ||
| * Schema result cache. | ||
| */ | ||
| protected static ?array $schema = NULL; | ||
|
|
||
| /** | ||
| * Handler instances cache. | ||
| */ | ||
| protected static ?array $handlers = NULL; | ||
|
|
||
| /** | ||
| * Get the generated schema (cached). | ||
| */ | ||
| protected function getSchema(): array { | ||
| if (static::$schema === NULL) { | ||
| $config = Config::fromString('{}'); | ||
| $prompt_manager = new PromptManager($config); | ||
| static::$handlers = $prompt_manager->getHandlers(); | ||
|
|
||
| $generator = new SchemaGenerator(); | ||
| static::$schema = $generator->generate(static::$handlers); | ||
| } | ||
|
|
||
| return static::$schema; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Static caching may cause test isolation issues.
The static properties $schema and $handlers (Lines 33, 38) persist across test methods and potentially across test classes in the same process. If a test modifies the schema or handlers, subsequent tests would see the modified state. Consider using setUpBeforeClass() and tearDownAfterClass() to manage the lifecycle, or document that these tests must not modify the cached data.
♻️ Suggested improvement for test isolation
+ public static function tearDownAfterClass(): void {
+ static::$schema = NULL;
+ static::$handlers = NULL;
+ parent::tearDownAfterClass();
+ }📝 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
| /** | |
| * Schema result cache. | |
| */ | |
| protected static ?array $schema = NULL; | |
| /** | |
| * Handler instances cache. | |
| */ | |
| protected static ?array $handlers = NULL; | |
| /** | |
| * Get the generated schema (cached). | |
| */ | |
| protected function getSchema(): array { | |
| if (static::$schema === NULL) { | |
| $config = Config::fromString('{}'); | |
| $prompt_manager = new PromptManager($config); | |
| static::$handlers = $prompt_manager->getHandlers(); | |
| $generator = new SchemaGenerator(); | |
| static::$schema = $generator->generate(static::$handlers); | |
| } | |
| return static::$schema; | |
| } | |
| /** | |
| * Schema result cache. | |
| */ | |
| protected static ?array $schema = NULL; | |
| /** | |
| * Handler instances cache. | |
| */ | |
| protected static ?array $handlers = NULL; | |
| /** | |
| * Get the generated schema (cached). | |
| */ | |
| protected function getSchema(): array { | |
| if (static::$schema === NULL) { | |
| $config = Config::fromString('{}'); | |
| $prompt_manager = new PromptManager($config); | |
| static::$handlers = $prompt_manager->getHandlers(); | |
| $generator = new SchemaGenerator(); | |
| static::$schema = $generator->generate(static::$handlers); | |
| } | |
| return static::$schema; | |
| } | |
| public static function tearDownAfterClass(): void { | |
| static::$schema = NULL; | |
| static::$handlers = NULL; | |
| parent::tearDownAfterClass(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php around lines 30
- 54, The static caches static::$schema and static::$handlers used by
getSchema() can leak state between tests; modify the test class to initialize
these caches in a setUpBeforeClass() (creating Config, PromptManager,
SchemaGenerator and populating static::$handlers/static::$schema) and clear them
in tearDownAfterClass() (setting static::$schema and static::$handlers back to
NULL) so each test run starts from a known state, or remove the statics and make
getSchema() return a fresh schema/handlers per call to ensure test isolation.
Member
Author
|
Member
Author
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2306 +/- ##
==========================================
- Coverage 77.76% 77.66% -0.10%
==========================================
Files 117 114 -3
Lines 6206 6241 +35
Branches 44 0 -44
==========================================
+ Hits 4826 4847 +21
- Misses 1380 1394 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
AlexSkrypnyk
force-pushed
the
feature/2302-ai-agent-installer
branch
from
7224d71 to
258748b
Compare
Member
Author
|
This comment has been minimized.
This comment has been minimized.
Member
Author
|
Member
Author
|
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.vortex/installer/src/Command/InstallCommand.php:
- Around line 319-326: The current code casts file_get_contents() to string so a
failure becomes an empty string and yields a misleading "Invalid JSON" message;
update the logic around $config_json/$config_option in InstallCommand (inside
the install command handler) to call file_get_contents($config_option) without
casting, check for === FALSE and if so write a clear error via $output->writeln
including the filename and/or error_get_last() info, then return
Command::FAILURE; keep the existing json_decode($config_json, TRUE) check but
ensure file-read errors are handled before attempting json_decode.
In @.vortex/installer/src/Prompts/Handlers/AbstractHandler.php:
- Around line 65-106: The dependsOn() method currently returns the PHP constant
NULL (uppercase); update the method in AbstractHandler::dependsOn to return the
lowercase null to follow PHP conventions and style (change "return NULL;" to
"return null;") while leaving envName() and type() as-is.
In @.vortex/installer/src/Prompts/PromptManager.php:
- Around line 576-581: The prompt() method currently indexes $this->handlers
using $handler_class::id() without confirming the key exists (mirroring the
earlier inconsistency with args()), so add a guard at the start of prompt():
compute $id = $handler_class::id() and check isset($this->handlers[$id]) (or
array_key_exists) and handle the missing handler consistently with args()—for
example throw a clear InvalidArgumentException or return a defined
fallback—before using $this->handlers[$id] and calling
$handler->type()->promptFunction().
In @.vortex/installer/src/Schema/ConfigValidator.php:
- Around line 199-201: The call in validateType to $handler->options([]) passes
an empty context so conditional options are ignored; update validateType to pass
the current collected responses/context into $handler->options(...) (e.g.,
$this->responses or the class property that stores answers) instead of an empty
array so HandlerInterface implementations can evaluate context-dependent options
during validation; ensure you reference the existing class property used
elsewhere for responses and keep the same method signature of validateType and
HandlerInterface::options.
In @.vortex/installer/src/Schema/SchemaGenerator.php:
- Around line 35-41: The prompt builder in SchemaGenerator is calling static
handler methods via the instance (handler->envName() and
handler->description([])) which can trigger deprecation warnings; change these
to static calls consistent with id() by calling the methods on the class (e.g.,
handler::envName() and handler::description([])) — update the array entries
where 'env' and 'description' are set to use handler::envName() and
handler::description([]) instead of the instance arrow syntax so all static
handler methods are invoked statically.
In @.vortex/installer/src/Utils/Normalizer.php:
- Around line 7-13: Replace the generic installer header docblock with a concise
description of the Normalizer utility: state that the Normalizer class provides
methods to normalize installer configuration values (e.g., sanitizing input,
normalizing paths, and normalizing environment keys) and outline its
responsibilities and typical usage; update the docblock that documents the
Normalizer class/namespace (Normalizer class in the Utils namespace) to include
purpose, public API summary, and any important side effects so readers
understand why this class exists and how to use it.
In @.vortex/installer/tests/Functional/Command/SchemaValidateCommandTest.php:
- Around line 46-56: In testSchemaOutputIsValidJson(), replace the plain
json_decode($output, TRUE) call with json_decode($output, true, 512,
JSON_THROW_ON_ERROR) (or equivalent) so parsing throws a JsonException on
failure; update the test to either let the exception fail the test or catch
JsonException for a clearer assertion message; reference the json_decode call in
testSchemaOutputIsValidJson and the use of applicationGetOutput() to locate
where to apply the change.
In @.vortex/installer/tests/Unit/Prompts/PromptTypeTest.php:
- Around line 76-80: Rename the test method to match the API being tested:
change the method name from testFromInvalidString to testTryFromInvalidString so
it clearly reflects that it is asserting behavior of PromptType::tryFrom; update
the method declaration (public function testFromInvalidString(): void) to public
function testTryFromInvalidString(): void and ensure any references to the old
name (if used by annotations or callers) are updated accordingly.
In @.vortex/installer/tests/Unit/Schema/ConfigValidatorTest.php:
- Around line 97-108: Replace hardcoded 'services' with the handler ID constant
Services::id() in testInvalidMultiselectValue: update the $config key and the
assertContains call that checks $error_prompts so both use Services::id()
instead of the literal string; locate the test method
testInvalidMultiselectValue and change occurrences of 'services' to
Services::id() to keep the test consistent with the handler identifier.
- Around line 110-121: The testMissingRequiredField currently only asserts that
$result has a 'resolved' key which is too weak; change the assertions after
$result = $this->validator->validate($config, $this->handlers) to explicitly
verify required-field behavior: assert that $result contains an 'errors' entry
and that those errors include the expected missing required fields (e.g. Name
and/or HostingProvider), and also assert that $result['resolved'] contains the
expected default values for fields that should be filled by defaults
(referencing $result, 'resolved', and the validator call
$this->validator->validate). Ensure you use assertArrayHasKey/assertContains or
equivalent PHPUnit assertions against $result['errors'] and $result['resolved']
rather than only checking that 'resolved' exists.
In @.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php:
- Around line 31-55: The test uses static caches static::$schema and
static::$handlers inside getSchema(), which can leak state across tests; change
getSchema() to avoid static state by removing those static properties and
returning a freshly generated schema each call (instantiate Config via
Config::fromString('{}'), PromptManager, call $prompt_manager->getHandlers() and
pass to new SchemaGenerator()->generate(...) inside getSchema()), or
alternatively clear static::$schema and static::$handlers in the test teardown;
update references to getSchema(), static::$schema, and static::$handlers
accordingly so tests do not share state.
258748b to
04fc605
Compare
Member
Author
|
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.vortex/installer/src/Prompts/Handlers/DatabaseImage.php (1)
43-55:⚠️ Potential issue | 🟡 MinorGuard shouldRun() against missing response to match established pattern.
DatabaseImage::shouldRun() directly accesses
$responses[DatabaseDownloadSource::id()]without anisset()guard. All other handlers with dependencies (MigrationDownloadSource, HostingProjectName, ProfileCustom) consistently useisset()checks before accessing dependent responses. Add a guard to match this defensive pattern:Suggested fix
public function shouldRun(array $responses): bool { return isset($responses[DatabaseDownloadSource::id()]) && $responses[DatabaseDownloadSource::id()] === DatabaseDownloadSource::CONTAINER_REGISTRY; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/src/Prompts/Handlers/DatabaseImage.php around lines 43 - 55, DatabaseImage::shouldRun currently accesses $responses[DatabaseDownloadSource::id()] directly; add an isset() guard so it first checks that the key exists before comparing to DatabaseDownloadSource::CONTAINER_REGISTRY. Update the shouldRun method to return true only when isset($responses[DatabaseDownloadSource::id()]) && $responses[DatabaseDownloadSource::id()] === DatabaseDownloadSource::CONTAINER_REGISTRY to match the defensive pattern used by MigrationDownloadSource, HostingProjectName, and ProfileCustom.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.vortex/installer/src/Prompts/Handlers/DatabaseImage.php:
- Around line 43-55: DatabaseImage::shouldRun currently accesses
$responses[DatabaseDownloadSource::id()] directly; add an isset() guard so it
first checks that the key exists before comparing to
DatabaseDownloadSource::CONTAINER_REGISTRY. Update the shouldRun method to
return true only when isset($responses[DatabaseDownloadSource::id()]) &&
$responses[DatabaseDownloadSource::id()] ===
DatabaseDownloadSource::CONTAINER_REGISTRY to match the defensive pattern used
by MigrationDownloadSource, HostingProjectName, and ProfileCustom.
---
Duplicate comments:
In @.vortex/installer/src/Command/InstallCommand.php:
- Around line 310-325: The current handleValidate method casts
file_get_contents($config_option) to string which hides file read failures and
causes misleading "Invalid JSON" messages; update handleValidate to explicitly
check file existence and the return value of file_get_contents when
$config_option is a path: use is_file($config_option) and then call
file_get_contents($config_option), verify it did not return false, and if it did
write a clear error via $output->writeln and return Command::FAILURE before
attempting json_decode; reference the method name handleValidate, the constant
OPTION_CONFIG, the variables $config_option, $config_json and $user_config, and
the file_get_contents call when making the change.
In @.vortex/installer/src/Prompts/Handlers/AbstractHandler.php:
- Around line 72-99: The type() method currently calls $this->options([]) and
$this->default([]), which can misinfer when those methods depend on actual
responses; replace these empty-array calls with the real/available responses
(e.g. $this->options($this->responses ?? []) and $this->default($this->responses
?? [])) or pass null if there are legitimately no prior responses and update
options()/default() to accept null safely; update the type() logic (method name:
type()) to use those actual-response-aware calls rather than [] so options(),
default() and the PromptType selection reflect real input-dependent behavior.
In @.vortex/installer/src/Prompts/PromptManager.php:
- Around line 576-582: The prompt() method accesses
$this->handlers[$handler_class::id()] directly and can trigger a PHP
notice/undefined index; update prompt() to validate the handler key the same way
args() does (use array_key_exists or isset for $handler_class::id() in
$this->handlers) and, if missing, throw the same RuntimeException with a clear
message; then proceed to build $fn = '\\Laravel\\Prompts\\' .
$handler->type()->promptFunction() and call it with $this->args($handler_class,
NULL, $responses) so error handling is consistent with args().
In @.vortex/installer/src/Schema/ConfigValidator.php:
- Around line 99-101: The code calls $handler->options([]) and
validateType($handler, $value) with an empty context, which skips
context-dependent option validation; update the calls to pass the current
normalized/resolved responses into both $handler->options(...) and
validateType(...) (e.g. use the existing $responses array after
resolution/normalization) so options() receives the real response context and
validateType() validates against those runtime options for $value.
In @.vortex/installer/src/Schema/SchemaGenerator.php:
- Around line 35-45: Change the instance-style calls to the handler's static
methods to use static call syntax: replace uses of $handler->envName() and
$handler->description([]) with $handler::envName() and $handler::description([])
respectively (leave other entries like $handler::id(),
Normalizer::normalizeOptions($handler->options([])), $handler->default([]),
$handler->isRequired(), and $handler->dependsOn() unchanged).
In @.vortex/installer/src/Utils/Normalizer.php:
- Around line 7-14: Replace the incorrect copy-pasted docblock for the
Normalizer class with a concise description of its actual responsibility:
describe that Normalizer normalizes/standardizes installer input or
configuration values (or whatever the class actually does), update the summary
and description lines to reference "Normalizer" rather than "Installer
configuration", and ensure tags (e.g., `@package` DrevOps\VortexInstaller) remain
accurate or are adjusted to match the class purpose; locate the docblock
immediately above the final class Normalizer declaration and update it
accordingly.
In @.vortex/installer/tests/Functional/Command/SchemaValidateCommandTest.php:
- Around line 46-53: The testSchemaOutputIsValidJson test currently calls
json_decode($output, TRUE) which silently returns null on parse errors; update
the call to json_decode($output, TRUE, 512, JSON_THROW_ON_ERROR) and wrap it in
a try/catch for \JsonException so parse failures surface as test failures (or
rethrow/assert with the exception message). Ensure you reference the $output
variable and replace the existing json_decode invocation in
testSchemaOutputIsValidJson, asserting/propagating the \JsonException so
malformed JSON fails the test.
In
@.vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php:
- Around line 35-40: Remove the unnecessary AiCodeInstructions environment setup
from the container_registry test case: in the test entry keyed 'db download
source, container_registry' remove the Env::put(AiCodeInstructions::envName(),
Env::TRUE) line because AiCodeInstructions is not used by DatabaseDownloadSource
or DatabaseImage; leave the Env::put calls for DatabaseDownloadSource::envName()
and DatabaseImage::envName() intact so the container_registry scenario still
tests the intended handlers.
In @.vortex/installer/tests/Unit/Prompts/PromptTypeTest.php:
- Around line 76-79: Rename the test method testFromInvalidString to reflect
that it asserts behavior of PromptType::tryFrom; update the method name to
something like testTryFromInvalidString (or test_tryFromInvalidString) so the
test name matches the assertions that call PromptType::tryFrom('invalid'),
PromptType::tryFrom(''), and PromptType::tryFrom('TEXT').
In @.vortex/installer/tests/Unit/Schema/ConfigValidatorTest.php:
- Around line 97-108: Update testInvalidMultiselectValue to use the Services
handler ID constant instead of the literal 'services': replace the hardcoded
'services' in the $config array with Services::id() so the test uses the same
identifier as other tests; ensure the rest of the assertions remain the same
(the call to $this->validator->validate($config, $this->handlers) and checking
$result['valid'] and error prompts) so the test remains consistent if the
handler ID changes.
- Around line 110-121: Update testMissingRequiredField to assert specific
expected defaults and error behavior after calling
$this->validator->validate($config, $this->handlers): verify that
$result['resolved'] exists and contains the expected default values for the
required fields (e.g., 'name' and 'hostingProvider' populated by their default()
handlers) and add assertions about $result['errors'] (either assert it's empty
if defaults resolve everything, or assert it contains the expected missing-field
entries) so the test verifies both resolution and error reporting; locate the
changes in the testMissingRequiredField method and use
$this->validator->validate and $this->handlers references to implement the
checks.
In @.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php:
- Around line 31-55: Tests leak static state because static::$schema and
static::$handlers are not reset; add a reset in the test lifecycle (e.g.,
implement tearDown() or tearDownAfterClass() in SchemaGeneratorTest) to set
static::$schema = NULL and static::$handlers = NULL so getSchema() always
rebuilds its fixtures between tests; reference the static properties
(static::$schema, static::$handlers) and the getSchema() helper when adding the
reset.
Member
Author
|
Member
Author
|
AlexSkrypnyk
force-pushed
the
feature/2302-ai-agent-installer
branch
from
04fc605 to
8dc69a2
Compare
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.vortex/installer/src/Command/InstallCommand.php:
- Around line 319-326: The code currently casts
file_get_contents($config_option) to string which hides failures (FALSE becomes
""), so update the logic around $config_json/file_get_contents to explicitly
detect a FALSE return (or pre-check is_readable/is_file) and emit a clear error
via $output->writeln indicating the config file could not be read, then return
Command::FAILURE before calling json_decode; do not rely on the subsequent
json_decode($config_json, TRUE) to surface file-read errors for $user_config
validation.
In @.vortex/installer/src/Prompts/Handlers/AbstractHandler.php:
- Around line 72-99: The type() method infers prompt kind using options([]) and
default([]) which can misidentify handlers whose options/defaults depend on
response context; update type() to accept an optional $responses parameter (and
thread it into calls to options($responses) and default($responses)) so
inference is response-aware, or if changing signature is unacceptable, document
the current best-effort limitation clearly in AbstractHandler::type() and
related docs; reference the type(), options(), default(), and PromptType symbols
when making the change so callers/implementations can be updated to pass
$responses where available.
In @.vortex/installer/src/Prompts/PromptManager.php:
- Around line 576-582: The prompt() method directly indexes $this->handlers
using $handler_class::id() which can trigger a PHP error for invalid handlers;
to fix, perform the same validation used in args() (use
array_key_exists($handler_class::id(), $this->handlers)) and if the key is
missing throw the same RuntimeException with a clear message before resolving
$handler and calling the prompt function so prompt() and args() handle invalid
handler classes consistently.
In @.vortex/installer/src/Schema/ConfigValidator.php:
- Around line 199-222: The validator currently calls $handler->options([])
inside validateType which ignores context-dependent option lists (e.g.,
DatabaseDownloadSource::options filters by HostingProvider); change validateType
to pass the validator's accumulated responses/context array into
HandlerInterface::options instead of an empty array so conditional options are
evaluated correctly (update the call in validateType and any callers to use the
existing responses/answers property or parameter that stores prior responses).
In @.vortex/installer/src/Utils/Normalizer.php:
- Around line 7-14: The class docblock above the final class Normalizer is
incorrect/copy-pasted; update it to describe the Normalizer's actual
responsibility (e.g., normalizing installer/input values, paths, or
configuration data) and include correct `@package` and brief summary that match
the Normalizer class purpose and behavior (refer to class Normalizer and any
methods within it to craft the accurate description).
In @.vortex/installer/tests/Functional/Command/SchemaValidateCommandTest.php:
- Around line 46-57: In testSchemaOutputIsValidJson(), replace the
json_decode($output, TRUE) call with a JSON_THROW_ON_ERROR-enabled decode (e.g.
json_decode($output, TRUE, 512, JSON_THROW_ON_ERROR)) and either wrap it in a
try/catch for JsonException to call $this->fail(...) with the exception message
or allow the exception to surface so the test fails with an explicit parse
error; update the reference to $schema accordingly so the subsequent assertions
(assertIsArray, assertArrayHasKey, assertNotEmpty) operate on the decoded
result.
In @.vortex/installer/tests/Unit/Prompts/PromptTypeTest.php:
- Around line 76-80: Rename the test method testFromInvalidString to
testTryFromInvalidString to match the behavior being tested
(PromptType::tryFrom); update the method declaration name and any
references/calls to this test in the test suite so the new name is used
consistently (look for testFromInvalidString and replace with
testTryFromInvalidString).
In @.vortex/installer/tests/Unit/Schema/ConfigValidatorTest.php:
- Around line 97-108: Replace the hardcoded 'services' string in
testInvalidMultiselectValue with the handler ID constant (use Services::id())
and add the corresponding import (use
DrevOps\VortexInstaller\Prompts\Handlers\Services;) so the assertions reference
the handler constant instead of a literal; update both occurrences (the config
key and the asserted prompt id) to use Services::id() and keep the rest of the
test logic unchanged.
- Around line 110-121: The test testMissingRequiredField currently only checks
that $result contains a 'resolved' key; instead assert concrete validation
behavior by calling $this->validator->validate($config, $this->handlers) and
then: verify 'errors' is present and either empty (if required fields have
defaults) or contains the expected missing-field keys, and verify 'resolved'
contains the expected default values for 'Name' and 'HostingProvider' (use
assertions like assertArrayHasKey/assertEmpty/assertEquals against
$result['resolved'] and $result['errors']). This uses the existing validate
method on $this->validator and the $this->handlers to confirm defaults are
applied or appropriate errors are returned.
In @.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php:
- Around line 31-55: Add a tearDownAfterClass method to clear the static caches
to avoid cross-test pollution: reset SchemaGeneratorTest::$schema and
SchemaGeneratorTest::$handlers to NULL in tearDownAfterClass so tests remain
isolated (refer to the static properties $schema and $handlers and the getSchema
method where they are set); implement the method as a public static function
tearDownAfterClass(): void that sets both static::$schema = NULL and
static::$handlers = NULL.
|
Member
Author
|
Member
Author
|
Member
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2302
Summary
Make the Vortex installer AI-agent friendly by adding machine-readable prompt
discovery (
--schema), config validation (--validate), and agent instructions(
--agent-help).An AI agent can now:
--schemato discover all prompts, types, options, defaults, and dependencies as JSON--validate --config=config.jsonto check the config before installing--no-interaction --config=config.jsonto install without promptsNew options on
InstallCommand--schema— outputs a JSON manifest of all installer prompts with theirid, env var name, type, label, description, options, default, required flag,
and dependency conditions
--validate— validates a--configJSON file against the schema andreturns errors, warnings, and resolved values (config merged with defaults)
--agent-help— outputs plain-text instructions for AI agents describingthe full workflow
New classes
Prompts\PromptTypetext,select,multiselect,confirm,suggest, etc.)Schema\SchemaGeneratorSchema\ConfigValidatorUtils\Normalizer[{value, label}]formatHandler changes
type(): PromptTypetoHandlerInterface— auto-inferred from handlerproperties in
AbstractHandler(no overrides needed)dependsOn(): ?arraytoHandlerInterface— overridden in 7 handlerswith conditional dependencies (e.g.,
DatabaseDownloadSourcedepends onProvisionType=database)envName(): stringtoHandlerInterface— returns theVORTEX_INSTALLER_PROMPT_*env var name for each handlerPromptManager::runPrompts()to dispatch viaPromptTypeenuminstead of hard-coded Laravel Prompts function calls
Test coverage
PromptTypeTest— enum cases,promptFunction()mapping,from()/tryFrom()AbstractHandlerTypeTest— data provider verifying type inference for all 37 handlersSchemaGeneratorTest— schema structure, required fields, options format,excluded handlers, env name format, handler count sync
ConfigValidatorTest— valid/invalid configs, dependency matrix (met/unmet ×provided/missing), env var key support, defaults resolution
SchemaValidateCommandTest— 25 functional tests using 17 JSON fixture filescovering valid configs, invalid values, broken JSON, empty files, warnings,
defaults, edge cases, and
--agent-helpoutputTest plan
composer lintpasses (phpcs, phpstan level 7, rector)composer test— all unit tests pass--schema,--validate,--agent-helppass--schema, feed to an AI agent, validate generated config with--validate, run install with--no-interaction --configSummary by CodeRabbit
New Features
Improvements
Tests