feat!: v1.0.0 — Graycore CI, FieldDescriptor schema, demo-verified release#1
Open
DavidLambauer wants to merge 38 commits intomainfrom
Open
feat!: v1.0.0 — Graycore CI, FieldDescriptor schema, demo-verified release#1DavidLambauer wants to merge 38 commits intomainfrom
DavidLambauer wants to merge 38 commits intomainfrom
Conversation
Documents the two-interface split (AiServiceConfigurationInterface vs AiServiceInterface), the stored-config data flow, and how to register a new AI backend. Flags the README's mis-attributed API example.
Captures the four locked brainstorming decisions (full release scope, getSupportedModels method, FieldDescriptor schema, one-off tag) and spells out CI, tests, release mechanics, and demo smoke test.
12 tasks decomposed into TDD-style steps with exact file paths, full code, and commit boundaries. Task 7 consolidates the breaking interface change with the 11 service rewrites and admin form update into a single atomic commit so the repo never lands in a broken state.
Packagist convention is to derive the version from git tags, not a hardcoded composer.json field. Leaving it in triggers a warning that composer validate --strict escalates to a failure. Task 12 already handles tagging as the source of truth.
Packagist derives the version from git tags (see Task 12). Leaving a hardcoded version field triggers a warning that composer validate --strict escalates to a failure.
Reviewer flagged CHANGELOG hygiene (Unreleased + compare-links + Keep-a-Changelog preamble), .gitattributes for export-ignore of dev paths, and a LICENSE split decision. All Minor, all batched for the pre-tag polish commit.
Graycore's supported-version action defaults to project: magento-open-source. Without this override the check-extension matrix tested against Magento Open Source versions, not the Mage-OS versions this module is actually built against.
Replaces the Laravel-style array_first() with native array_key_first() and guards every boundary (null scope value, non-string config, failed json_decode, non-array rows, missing code key) before constructing AiService DTOs. Covered by 4 new unit tests.
…tract Adds a parametrised negative test for the 'malformed row' and 'bad row shape' guards so a future refactor can't silently remove them. Also documents on AiServiceSelectorInterface that results are returned in insertion order — the positional assertions in the unit and integration tests depend on that being a real contract, not an incidental behaviour.
BREAKING CHANGE: AiServiceConfigurationInterface::getConfigurationTemplate() is removed. Implementers now return FieldDescriptor[] from getConfigurationFields() and (optionally) a model list from getSupportedModels(). Admin form phtml now renders fields by type from a JSON schema rather than substituting HTML template strings. Storage format is unchanged, so the AiServiceSelector consumer API is fully backwards compatible.
Code review follow-ups from Task 7: - escapeHtml() helper wraps every schema-sourced string emitted into the DOM via innerHTML-style concatenation. Whitelists input types to password/text. - Select-field rehydration preserves stored values that are no longer in the options list by prepending a synthetic '(legacy)' option, preventing silent data loss when model lists get refreshed. - Services block is now final and validates that each injected service implements AiServiceConfigurationInterface at construction time, with the closure in getServicesSchemaJson gaining the corresponding type hint.
Review follow-up on Task 9: - Fixture delete now lives in tearDown() so a failed assertion cannot leak config rows into the test DB. - Cache-clean uses Magento\Framework\App\Config directly rather than calling ->clean() on a ScopeConfigInterface-typed variable, which is not a method on that interface.
AiService gains final, and AiServiceInterface::getConfiguration() gets an array-shape docblock. registration.php only needed the declare line.
…GELOG hygiene composer.json support URLs and CHANGELOG compare-links now point at the real mage-os-lab org. CHANGELOG gains Keep-a-Changelog preamble, [Unreleased] section, compare-link footer, and refreshes the 1.0.0 body to reflect review follow-ups (guard coverage, final Block, admin-form hardening). Release date bumped to 2026-04-21. .gitattributes keeps .github/, docs/, src/Test/, phpcs.xml.dist, phpunit.xml.dist, and CLAUDE.md out of the tarball Packagist serves from a tagged release.
Graycore's check-extension reusable workflow defaults magento_repository to https://mirror.mage-os.org/, but that mirror only proxies magento/* packages. The mage-os/project-community-edition metapackage (emitted by supported-version) is published to https://repo.mage-os.org/, so composer create-project fails with "Could not find package mage-os/project-community-edition" and all four check-extension jobs abort in <15s with zero test signal. Overriding magento_repository on the reusable workflow call points Composer at the correct mage-os Composer repository. Staying pinned at @v5.1.0 (supply-chain) - the matrix itself is fine, only the repo URL was wrong.
Move Test/ to package root so Graycore's PHPUnit test-discovery (...vendor/<pkg>/Test/Unit) locates our suites. Add the mage-os composer repo so the coding-standard job's composer install at module root can resolve magento/framework and mage-os/magento- coding-standard. Namespace (MageOS\\AiBase\\Test\\*) is unchanged; only the autoload- dev path mapping shifts from src/Test/ to Test/.
coding-standard job installs magento/magento-coding-standard + magento/php-compatibility-fork at module root, which pulls in magento/framework. That lives on mirror.mage-os.org (serves magento/* namespace), not repo.mage-os.org (serves mage-os/*). Adding both repos with mirror listed first.
With mirror.mage-os.org added, composer require pulls in magento/composer-dependency-version-audit-plugin as a transitive dependency. Composer 2.2+ requires explicit allow-plugins opt-in, and the coding-standard job was aborting on the interactive prompt. Also pre-allows dealerdirect/phpcodesniffer-composer-installer (used by magento-coding-standard) and magento/composer-root-update-plugin so future installs don't re-prompt.
v6.0.0 adds explicit MageOS 2.2.0 to the supported-version matrix, which matches our target demo environment. Also potentially fixes two v5.1.0 bugs: coding-standard hardcoding vendor/magento/* paths instead of handling mage-os-namespaced packages, and integration_test referencing undefined matrix.services when include_services is false. Re-pinning to a tag (not @main) keeps supply-chain risk bounded.
v6.0.0 introduced a template-validation regression in the integration_test job (line 202 references an unset include_services value) and didn't fix the coding-standard Magento2-sniff-not-found issue either. v5.1.0 had 3/5 jobs passing; v6.0.0 had the same 2 failures plus one new one. Net regression. Reopening the upstream-compat question as a follow-up; v5.1.0 ships.
Author
CI status — parked pending upstream Graycore fixesFiled both blocking failures upstream as reproducible bugs:
Both reproduce on any Mage-OS module that consumes the Graycore reusable workflow; neither is specific to this PR. This PR sits until Graycore ships a patch that makes those two jobs pass against mage-os packaging. In the meantime:
Release path is blocked only by the CI infra, not by any code correctness question. |
Author
|
Added GitHub Community Standards in the latest 8 commits (
Design and plan: Before merge: enable Private Vulnerability Reporting in repo Settings > Code security and analysis, otherwise the link in |
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.
Summary
Cuts
mage-os/module-ai-baseto a production-ready v1.0.0:check-extensionreusable workflow pinned atv5.1.0, matrix-targeted atproject: mage-osso every PR gets tested against the current Mage-OS supported-version matrix.AiServiceConfigurationInterface::getConfigurationTemplate(): string→getConfigurationFields(): FieldDescriptorInterface[]+getSupportedModels(): array. ElevenAiServices/*classes rewritten to the new schema; stored-config on-disk format unchanged soAiServiceSelectorInterfaceconsumers are fully backwards-compatible.escapeHtml()on every schema-sourced value, input-type whitelist, and legacy-stored-value preservation when the model list has changed.Block\Adminhtml\Configuration\Servicesmarkedfinalwith runtimeinstanceofguard.Model\AiServiceSelectoragainst null scope values / malformed JSON / malformed rows / bad row shapes. All four covered by unit tests.AiServiceSelector,FieldDescriptor, and the parametrisedServicesTesthitting all eleven providers. One integration test covers full config round-trip throughScopeConfigInterfacewith failure-safe cleanup.Design + plan docs:
docs/plans/2026-04-20-done-done-design.mdanddocs/plans/2026-04-20-done-done-plan.md.CI status
3 of 5 jobs pass. The two failing jobs are pre-existing Graycore v5.1.0 infrastructure issues unrelated to this module's code — they would fail on any Mage-OS module using the
@v5.1.0reusable workflow.compute-matrixcheck-extension / compile-extensiondi.xml+etc/*configs compile cleanly on a fresh Mage-OS 2.1.0 installcheck-extension / unit-test-extensioncheck-extension / coding-standardmagento/magento-coding-standardaliases tomage-os/magento-coding-standard; Graycore hardcodes thevendor/magento/*path ininstalled_paths, so theMagento2sniff can't be found). v6.0.0 didn't fix this; v5.1.0 is still the least-broken tag. Locallycomposer validate --strictpasses andphpcs --standard=Magento2 src/works through the auto-installed paths.check-extension / integration_testmatrix.servicesin its reusable template at line 202 even wheninclude_services: false(the default fromsupported-version). Fails in 2s before any setup runs. v6.0.0 introduced a template-validation regression that made this worse, so we stayed on v5.1.0.Upstream follow-up: both issues should be filed against
graycoreio/github-actions-magento2for mage-os-ecosystem support. This PR ships with the test coverage that matters (unit + DI compile) on green, and documents the infra failures as known-issues.Test plan
check-extension / compile-extensionpasses on Mage-OS 2.1.0 in CI.check-extension / unit-test-extensionpasses (21 tests, 154 assertions)./Users/david/Herd/mage-os-typesense(Mage-OS 2.2.0), admin UI loads all 11 service buttons, save round-trip works (password masked, select correctly rehydrated),AiServiceSelectorInterface::getAll()returns 2 configured services with correct codes + configuration,getByCode()filters correctly.composer validate --no-check-publish --strictexits 0.Out of scope for v1.0.0
release-pleaseworkflow (deferred).<preference>.coding-standard+integration_testjob compatibility with mage-os-namespaced packages — upstream fix needed.