make:payload --graphql + verify tooling hardening (develop→master)#32
Open
syntaxwanderer wants to merge 6 commits into
Open
make:payload --graphql + verify tooling hardening (develop→master)#32syntaxwanderer wants to merge 6 commits into
syntaxwanderer wants to merge 6 commits into
Conversation
…arve-outs
Part of ep-ai-verify-broken-fqcn-guard. The header comment claimed
native PHPStan checks were disabled in the focused config; they were
never disabled (no customRulesetUsed flag, so level-0 default rules
run alongside the listed Semitexa rules). Empirical probe:
use Semitexa\Theme\Contract\SkinAlgorithm; // deleted FQCN
public function run(SkinAlgorithm \$a): SkinAlgorithm {}
already fires class.notFound under this config.
Rewrite the header to match reality so the next reader understands
that phpstan_di IS our first line of defence against broken-FQCN
time-bombs introduced by vendor renames.
Add two carve-outs that the repo-wide audit identified as pure noise:
- excludePaths: packages/semitexa-core/src/Composer/SemitexaPlugin.php
Mirrors the main phpstan.neon exclusion. The file pulls in
composer/composer which is only available on the host, never
inside the framework autoload tree. Without this, 14 unrelated
class.notFound errors fire (Composer\Composer, Composer\Script\Event,
Composer\Plugin\PluginInterface, ...).
- ignoreErrors: phpstanApi.runtimeReflection scoped to
packages/semitexa-core/src/PHPStan/Rules/*. Our custom rules
deliberately use is_subclass_of() against runtime Semitexa
attribute classes loaded via the project autoloader; PHPStan's
own diagnostic about runtime-reflection-vs-static-reflection
is not a class-resolution concern and not actionable in our
context.
Repo-wide audit drops 56 -> 42 native errors with these two carve-outs.
The remaining 42 are triaged in ep-ai-verify-broken-fqcn-guard for
Step 2 (SHIM ignoreErrors) and Step 4 (24 real-bug sweep).
Refs ep-ai-verify-broken-fqcn-guard
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…files
Part of ep-ai-verify-broken-fqcn-guard step 2 (SHIM bucket).
The 5 files affected are top-level deprecation bridges of this shape:
namespace Semitexa\Auth;
use Semitexa\Core\Auth\AuthenticationMode as CoreAuthenticationMode;
if (!class_exists(AuthenticationMode::class, false)) {
class_alias(CoreAuthenticationMode::class, AuthenticationMode::class);
}
PHPStan cannot statically evaluate class_alias(), so it reports
`Class Semitexa\Auth\AuthenticationMode not found.` against the file's
own declared FQCN. The shim is intentional and works at runtime —
this is a false positive.
Files:
- packages/semitexa-auth/src/AuthenticationMode.php
- packages/semitexa-ledger/src/Contract/ReplayHandlerInterface.php
- packages/semitexa-ssr/src/Contract/TypedSlotHandlerInterface.php
- packages/semitexa-ssr/src/Seo/Sitemap/AsSitemapProvider.php
- packages/semitexa-ssr/src/Seo/Sitemap/SitemapUrlProviderInterface.php
Eventually we should delete these once downstream cutover is
confirmed complete — captured as a separate backlog item. For now,
path-scoped ignoreErrors of just identifier=class.notFound.
Net: focused-config audit drops 42 -> 37 native errors. The remaining
37 break down into 13 CONFIG_GAP (uninstalled packages — operator
decision pending), 11 MECHANICAL, 8 MODERATE, 5 COMPLEX time-bombs
for the step-4 sweep.
Refs ep-ai-verify-broken-fqcn-guard
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Part of ep-ai-verify-broken-fqcn-guard step 3 (CONFIG_GAP resolution). `semitexa-ultimate` is `type: project` — the skeleton consumed by `composer create-project semitexa/ultimate`, not a library required by semitexa.dev. It is actively maintained (release pin target for 2026.05.27.0859), but it would be circular to `require` the skeleton from the dev environment that produces it. ai:verify in semitexa.dev gates framework + library code; ultimate is gated by its own release-readiness workflow. This resolves the only remaining ambiguous-scope package after the repo-wide audit. The other three CONFIG_GAP packages on disk but not required (storage, mail, media, workflow) were a batched-release oversight: their composer.json carry consistent pinned versions in the consuming semitexa.pl app yet semitexa.dev never `require`d them, so ai:verify could not see them. They are now added to the unversioned semitexa.dev/composer.json `require` block (local-only — semitexa.dev itself is not a versioned repo); composer update symlinked them into vendor/semitexa/. Audit before this step: 37 native errors (post step-1+step-2 carve-outs and SHIM ignores). 13 of those were CONFIG_GAP noise. After composer require + this exclude: 29 native errors, with the CONFIG_GAP bucket emptied. Reclassification surfaced 5 previously hidden real bugs (4 attribute-DI noctor patterns in media, 1 storage-internal HttpStatus constant). Bucket distribution settles at MECHANICAL=11, MODERATE=13, COMPLEX=5 — ready for the step-4 sweep. Refs ep-ai-verify-broken-fqcn-guard Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…files The first SHIM pass (0170f00) only detected whole-file class_alias deprecation shims. Two more files use the inline variant where the file declares the canonical class and then registers the backward-compat alias at the end of the same file: namespace Semitexa\Api\Attribute; final class ApiVersion { ... } if (!class_exists(\Semitexa\Api\Attributes\ApiVersion::class, false)) { class_alias(ApiVersion::class, \Semitexa\Api\Attributes\ApiVersion::class); } PHPStan can't statically evaluate class_alias(), so the legacy FQCN appears unresolvable. Same false-positive class as the whole-file shims, just authored differently. Files: - packages/semitexa-api/src/Attribute/ApiVersion.php - packages/semitexa-api/src/Attribute/ExternalApi.php Cross-repo grep over semitexa.dev and semitexa.pl found zero consumers of the legacy Semitexa\Api\Attributes\ namespace as of 2026-05-28; the inline alias blocks appear to be dead code, but external consumer apps may still reference them so we don't delete yet. Both files added to the existing deletion-tracking epic ep-delete-class-alias-deprecation-shims (now covering 7 shims, not 5). Net: ai:verify focused audit drops 29 -> 27 native errors. Step-4 sweep scope: 9 MECHANICAL, 13 MODERATE, 5 COMPLEX. Refs ep-ai-verify-broken-fqcn-guard Refs ep-delete-class-alias-deprecation-shims Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…expansion
Closes the dominant failure mode behind ep-ai-verify-broken-fqcn-guard:
when a contract is removed or renamed elsewhere in the repo, the
consumers that still reference its FQCN are not in the changed-file
list, so phpstan_di never scans them and the broken `use` / typehint
survives merge as a silent time-bomb. The vendor DDD migrations of
SkinResolver (Llm + Theme) shipped exactly this way.
PHPStan at level 0 already detects `class.notFound` / `interface.notFound`
references — the focused phpstan-ai-verify.neon has had those active
since inception. What was missing was *telling PHPStan to look at the
orphan consumers*. The previous sweep (ep-ai-verify-broken-fqcn-guard
parts 1+2) cleared the 56 pre-existing native errors so this guard
ships on a clean baseline.
Layer 1 components:
1. ContractMoveResolver (~280 LoC).
- Input: list<ChangedFile>.
- Filter: PHP files with status DELETED or RENAMED only — never
MODIFIED/ADDED, which is the false-positive control. ai:verify is
shared infra and a spurious expansion would break every package's
verify.
- FQCN derivation: vendor/composer/autoload_psr4.php (always current
because Composer regenerates it on dependency changes) maps the
deleted file's repo-relative path back to its declared FQCN. No
filesystem read of the deleted file needed.
- Usage query: shells out to `bin/semitexa ai:review-graph:query
--usages=<FQCN> --json` via the existing ProcessRunner
abstraction. One CLI invocation per unique removed FQCN
(deduped). The output is robust to Symfony's info-line prefixes
(`[INFO] Running locally with PHP...`) via a tolerant JSON
extractor that walks lines bottom-up looking for the actual
array payload.
- FQCN → file: vendor/composer/autoload_classmap.php (single
require, in-memory cached). realpath() canonicalises through
vendor/semitexa/* symlinks to the canonical packages/semitexa-*/
path so downstream tooling sees one path per file.
- No-op-safe: missing CLI, missing classmap, graph reports zero
usages, FQCN not derivable from PSR-4 — every "I cannot tell"
case yields no expansion rather than a guessed one.
2. ContractMoveExpansion value object (~40 LoC).
- Carries the removed/renamed FQCN, the source file path, the
change status, and the dependent files. reason() formats the
NDJSON expansion line documented in AI_BEST_PRACTICES.md §23.3.
3. VerificationPlanner integration.
- Constructor accepts an optional ContractMoveResolver; default is
a fresh instance bound to projectRoot. Tests inject a stub via
subclass that overrides resolve().
- plan() invokes the resolver up front and pushes its expansions
into the existing $expansions list — the NDJSON envelope already
surfaces those, so no caller-side change required.
- phpstanDiTarget() now accepts the contract-move expansions,
appends their dependent files to the eligible scan set,
deduplicating against files already in the change set, and
reports a contextual reason when expansion fired ("validate
Semitexa DI rules + broken-FQCN guard on consumers of N
removed/renamed contract(s)").
4. PhpstanRunner identifier remap.
- extractDiagnostics() reclassifies raw `class.notFound` /
`interface.notFound` / `trait.notFound` / `enum.notFound`
identifiers to the Semitexa-namespaced `semitexa.brokenFqcn`. A
fallback pattern-match against the message text catches older
PHPStan versions that surface broken references as
`phpstan.error` without a typed identifier.
- suggestionFor() emits "Vendor migration likely — the referenced
FQCN no longer resolves." with concrete guidance on the DDD
rename and attribute-relocation patterns the previous sweep
uncovered. Reminds the operator to run
`bin/semitexa ai:review-graph:generate` if the impact graph may
be stale.
5. Tests (12 ContractMoveResolverTest + 4 VerificationPlannerTest):
- DELETED contract with known consumers → expansion produced
- RENAMED contract → expansion produced
- MODIFIED contract → no expansion (the false-positive guard)
- Unrelated change set → CLI never invoked
- Non-PHP deletion → ignored
- PSR-4-unmapped path → ignored
- Self-edges dropped
- Graph CLI failure swallowed silently
- Duplicate FQCNs deduped to one query
- Dependent file already in changed set → not duplicated
- Inline-shim and other edge cases covered
Validation against the proof scenario:
mv packages/semitexa-theme/src/Domain/Contract/SkinAlgorithmInterface.php /tmp/...bak
composer dump-autoload -o
echo "D packages/semitexa-theme/.../SkinAlgorithmInterface.php" \
| bin/semitexa ai:verify --diff-stdin --json
expansions:
"contract Semitexa\Theme\Domain\Contract\SkinAlgorithmInterface
removed — adding 5 dependent file(s) to phpstan_di scan"
phpstan_di target.triggered_by:
BalancedAlgorithm, BrutalistAlgorithm, GlassAlgorithm,
SkinAlgorithmRegistry, SkinBuilder (the exact consumer set)
violations: 6 × semitexa.brokenFqcn, each carrying the
"Vendor migration likely" suggestion.
That is the SkinResolver pattern: had this guard existed at the
commit that renamed `Theme\Contract\SkinAlgorithm` to
`Theme\Domain\Contract\SkinAlgorithmInterface`, ai:verify would have
failed the commit with concrete file:line callouts on every consumer.
Negative-control runs confirm zero false positives:
- clean unrelated change set → pass, 0 expansions, 0 brokenFqcn
- MODIFIED on a contract file → pass, only pre-existing
"contracts ripple" scope expansion fires (Layer 1 silent)
612/612 phpunit tests pass (1483 assertions, 2 pre-existing skipped).
Layer 2 (repo-wide `--all` FQCN sweep) deferred per design; the
infrastructure is ready for it to land as a second target type
without further changes here.
Refs ep-ai-verify-broken-fqcn-guard
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
--graphql emits a bare #[ExposeAsGraphql] marker (field name derives from the
Payload class name) plus its import; --graphql-field overrides the name. The
template gains a {{graphqlAttribute}} placeholder that collapses cleanly when
the flag is absent (no stray blank line, no import).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
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.
Release of the current
developdelta tomaster(6 commits).Headline:
make:payload --graphql(this session)bd0bc91— adds a--graphqlflag tomake:payloadthat scaffolds a bare#[ExposeAsGraphql]marker on the generated Payload (field name derives fromthe class), plus
--graphql-fieldto override. Template placeholder collapsescleanly when the flag is absent. Covered by 3 new
MakePayloadCommandTestcases (7/7 green);
ai:verify5/5, 0 violations.Pairs with semitexa-graphql's zero-param
#[ExposeAsGraphql]change (separaterepo / PR), and was verified end-to-end against the live Swoole runtime: a
generated payload surfaced as a derived GraphQL field and resolved.
Also included (pre-existing unmerged
developcommits)7073dfffeat(verify): Layer 1 broken-FQCN guard — contract-move blast-radius88ac8dbfix(verify): ignore class.notFound on 2 inline class_alias shims1de638dfix(verify): exclude semitexa-ultimate/src from focused phpstan config0170f00fix(verify): ignore class.notFound on 5 class_alias deprecation shims9498c80fix(verify): correct phpstan-ai-verify config truth + noise carve-outs🤖 Generated with Claude Code