Feature enforcement

[distvan]

PHPDecide / Architecture decisions

What architectural decision(s) apply to this change?

• I ran composer phpdecide -- explain "<question>" --path <changed-file> for the key changed files
• I added/updated a decision in .decisions/ if this PR introduces a new rule or exception

Checks

• composer decisions:lint
• composer test

Notes for reviewers

• If you’re unsure whether a decision applies, try:
  • composer phpdecide -- explain "What rules apply here?" --path <file>

[Copilot]

Pull request overview

This PR introduces an initial “enforcement-ready” workflow to PHPDecide by adding an enforce CLI command that maps external analyzer findings (generic JSON, Semgrep JSON, PHPStan JSON) back to active decision IDs via rules.forbid, and adds example assets/docs to support CI integrations.

Changes:

• Added phpdecide enforce command + enforcement domain classes to load findings, match them to decisions, and emit text/JSON outputs.
• Added PHPUnit coverage for the new command and refactored test-only filesystem exceptions to a shared helper exception.
• Added Semgrep/PHPStan example assets, GitHub Actions example workflows, and documentation updates describing the enforcement mapping contract.

Reviewed changes

Copilot reviewed 27 out of 28 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/Decision/YamlDecisionLoaderTest.php	Reuses shared test filesystem exception instead of local class.
tests/CLI/ExplainCommandTest.php	Reuses shared test filesystem exception instead of local classes.
tests/CLI/EnforceCommandTest.php	Adds coverage for enforce across generic/Semgrep/PHPStan inputs + JSON output + UTF-16.
src/Enforcement/AnalyzerFinding.php	Introduces normalized finding DTO used across loaders/matching/output.
src/Enforcement/AnalyzerFindingLoader.php	Loads “generic” enforcement JSON findings.
src/Enforcement/SemgrepFindingLoader.php	Parses native Semgrep JSON into normalized findings.
src/Enforcement/PhpStanFindingLoader.php	Parses native PHPStan JSON into normalized findings.
src/Enforcement/JsonFileDecoder.php	Shared JSON decoding with UTF/BOM normalization for reports.
src/Enforcement/DecisionViolation.php	Wraps a decision + finding pair for reporting.
src/Enforcement/EnforcementMatchResult.php	Holds matched violations + unmapped findings + summary helpers.
src/Enforcement/DecisionViolationMatcher.php	Matches findings to active scoped decisions via rules.forbid.
src/CLI/EnforceCommand.php	Implements the enforce CLI command with text/JSON renderers.
bin/phpdecide	Registers the new EnforceCommand with the CLI application.
semgrep/rules/no-orm-in-order-domain.yaml	Adds example Semgrep rule ID intended to match a decision token.
semgrep/rules/no-business-logic-in-templates.yaml	Adds a second example Semgrep rule for templates.
examples/phpstan/README.md	Adds PHPStan example documentation + token/identifier contract.
examples/phpstan/no-orm-in-order-domain-report.json	Adds a checked-in native PHPStan JSON report example.
examples/github-actions/phpdecide-semgrep-enforce.yaml	Adds a Semgrep-based CI example for enforcement + rendering outputs.
examples/github-actions/phpdecide-phpstan-enforce.yaml	Adds a PHPStan-based CI example for enforcement + rendering outputs.
examples/github-actions/render-phpdecide-pr-comment.php	Adds a JSON→markdown renderer for PR comments/step summaries.
examples/github-actions/render-phpdecide-annotations.php	Adds a JSON→GitHub annotation commands renderer.
examples/fixtures/templates/order/show.html.twig	Adds allowed template fixture for Semgrep example.
examples/fixtures/templates/order/calculate_total.html.twig	Adds violating template fixture for Semgrep example.
examples/fixtures/phpstan/src/Order/OrderService.php	Adds violating PHPStan fixture file.
examples/fixtures/phpstan/src/Infrastructure/Persistence/Doctrine/OrderRecord.php	Adds allowed PHPStan fixture file.
docs/decision-file-anatomy.md	Documents the enforcement token matching contract.
README.md	Documents the enforcement workflow, formats, and example assets.
CHANGELOG.md	Describes the new enforcement workflow and example assets.

[Copilot]

The README links to sample decisions under .decisions/ (DEC-0003/4/5), but this PR doesn't add a .decisions directory (and those referenced files don't exist in the repo). As written, the links are broken and the documented enforce/CI workflows that rely on default .decisions will fail. Either add the referenced decision files under .decisions/, or update the docs to point to the actual checked-in location (and adjust the examples accordingly).

Suggested change

	- Active repo sample decision: [.decisions/DEC-0003.no-orm-in-order-domain.yaml](.decisions/DEC-0003.no-orm-in-order-domain.yaml)
	- Active repo sample decision: [.decisions/DEC-0004.no-business-logic-in-templates.yaml](.decisions/DEC-0004.no-business-logic-in-templates.yaml)
	- Active repo sample decision: [.decisions/DEC-0005.no-orm-in-order-domain-via-phpstan.yaml](.decisions/DEC-0005.no-orm-in-order-domain-via-phpstan.yaml)
	These are example decision files you would place under your own `.decisions/` directory:
	- Active repo sample decision: DEC-0003.no-orm-in-order-domain.yaml
	- Active repo sample decision: DEC-0004.no-business-logic-in-templates.yaml
	- Active repo sample decision: DEC-0005.no-orm-in-order-domain-via-phpstan.yaml

[Copilot]

This section references ../.decisions/DEC-0004... as a checked-in example, but .decisions isn't present in the repo and the linked file doesn't exist. Update the link/wording to match the actual example location, or add the referenced decision file under .decisions/ so the documentation stays accurate.

Suggested change

	- See [../.decisions/DEC-0004.no-business-logic-in-templates.yaml](../.decisions/DEC-0004.no-business-logic-in-templates.yaml) and [semgrep/rules/no-business-logic-in-templates.yaml](../semgrep/rules/no-business-logic-in-templates.yaml) for a second example pairing using the token `twig/business-logic`.
	- See [DEC-0004.no-business-logic-in-templates.yaml](DEC-0004.no-business-logic-in-templates.yaml) and [semgrep/rules/no-business-logic-in-templates.yaml](../semgrep/rules/no-business-logic-in-templates.yaml) for a second example pairing using the token `twig/business-logic`.

[Copilot]

These changelog entries claim new sample decision files were added under .decisions/ (e.g. DEC-0004/DEC-0005), but the PR doesn't include a .decisions directory or those files. Either add the referenced decision files, or adjust the changelog to reflect what was actually added so consumers aren't misled.

Suggested change

	- Added a second active sample decision/rule pair for template boundaries:
	- `.decisions/DEC-0004.no-business-logic-in-templates.yaml`
	- `semgrep/rules/no-business-logic-in-templates.yaml`
	- Added example Twig fixtures so the template-boundary rule has a concrete allowed/violating path pair:
	- `examples/fixtures/templates/order/show.html.twig`
	- `examples/fixtures/templates/order/calculate_total.html.twig`
	- Added checked-in PHPStan example assets for the native identifier mapping path:
	- `.decisions/DEC-0005.no-orm-in-order-domain-via-phpstan.yaml`
	- Added a second sample Semgrep rule for template boundaries:
	- `semgrep/rules/no-business-logic-in-templates.yaml`
	- Added example Twig fixtures so the template-boundary rule has a concrete allowed/violating path pair:
	- `examples/fixtures/templates/order/show.html.twig`
	- `examples/fixtures/templates/order/calculate_total.html.twig`
	- Added checked-in PHPStan example assets for the native identifier mapping path:

[Copilot]

This example README points to .decisions/DEC-0005.no-orm-in-order-domain-via-phpstan.yaml, but that file (and the .decisions/ directory) isn't present in the repo. Either add the sample decision under .decisions/ or update this documentation to reference the correct checked-in decision/example location.

Suggested change

	Matching decision and fixture files:
	- `.decisions/DEC-0005.no-orm-in-order-domain-via-phpstan.yaml`
	- `examples/fixtures/phpstan/src/Order/OrderService.php`
	- `examples/fixtures/phpstan/src/Infrastructure/Persistence/Doctrine/OrderRecord.php`
	Matching fixture files:
	- `examples/fixtures/phpstan/src/Order/OrderService.php`
	- `examples/fixtures/phpstan/src/Infrastructure/Persistence/Doctrine/OrderRecord.php`
	In a real project, you should also have a decision file under `.decisions/` whose `rules.forbid` array contains the token `phpstan.doctrine.orm`.

[Copilot]

This workflow runs phpdecide decisions:lint --require-any, which will fail in this repo because the PR does not include a .decisions/ directory with at least one .yaml decision file. Either commit the sample decisions under .decisions/ (as referenced in the README/CHANGELOG) or remove --require-any/create decisions as part of the workflow so the example is runnable.

Suggested change

	run: php ./bin/phpdecide decisions:lint --require-any
	run: php ./bin/phpdecide decisions:lint

[Copilot]

This workflow runs phpdecide decisions:lint --require-any, which will fail in this repo because the PR does not include a .decisions/ directory with at least one .yaml decision file. Either commit the sample decisions under .decisions/, or change the workflow to not require any decisions (or to create them) so the example works out of the box.

Suggested change

	- name: Initialize sample decisions for example workflow
	run: |
	mkdir -p .decisions
	cat > .decisions/DEC-0001-sample.yaml << 'EOF'
	id: DEC-0001
	title: Sample decision for PHPDecide GitHub Actions example
	status: accepted
	date: 2024-01-01
	scope: Example workflows
	decision: >
	This is a sample architectural decision record created by the
	example GitHub Actions workflow so that `phpdecide decisions:lint`
	with `--require-any` has at least one decision file to validate.
	EOF

[Copilot]

Pull request overview

Copilot reviewed 30 out of 32 changed files in this pull request and generated 2 comments.

[Copilot]

writeJson() wraps JsonException into an InvalidArgumentException, but execute() only catches exceptions around loading/matching — not around result/error rendering. If json_encode() fails (e.g., invalid UTF-8 in an analyzer message), the command will throw and crash instead of returning a structured error + Command::FAILURE. Consider catching Throwable around renderResult() / renderError() as well, and/or using JSON_INVALID_UTF8_SUBSTITUTE to make JSON output robust for CI consumers.

Suggested change

	$output->writeln(json_encode($payload, JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR));
	$output->writeln(json_encode(
	$payload,
	JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR | JSON_INVALID_UTF8_SUBSTITUTE
	));

[Copilot]

The changelog references example decision files under .decisions/…, but this PR adds them under examples/decisions/… (e.g. examples/decisions/DEC-0004.no-business-logic-in-templates.yaml and examples/decisions/DEC-0005.no-orm-in-order-domain-via-phpstan.yaml). Updating these paths will avoid confusing users who try to locate the assets described here.

[Copilot]

Pull request overview

Copilot reviewed 30 out of 32 changed files in this pull request and generated 2 comments.

[Copilot]

In this test helper, mkdir($decisionsDir) is not checked for failure. If directory creation fails (permissions, disk full, etc.), later assertions will fail with less actionable errors. Consider validating the return value (and is_dir) and throwing TestFilesystemException, consistent with other CLI tests.

Suggested change

	mkdir($decisionsDir);
	if (!mkdir($decisionsDir) && !is_dir($decisionsDir)) {
	throw new TestFilesystemException('Unable to create decisions dir: ' . $decisionsDir);
	}

[Copilot]

writeFile() ignores file_put_contents() failures. If a write fails, tests will proceed and may produce misleading failures later. Consider checking the return value and throwing TestFilesystemException (as done in DecisionsLintCommandTest).

Suggested change

	file_put_contents($path, $contents);
	$result = file_put_contents($path, $contents);
	if ($result === false) {
	throw new TestFilesystemException(sprintf('Failed to write file: %s', $path));
	}

[Copilot]

Pull request overview

Copilot reviewed 30 out of 32 changed files in this pull request and generated 1 comment.

[Copilot]

This test invokes the private EnforceCommand::writeJson() via ReflectionMethod without making it accessible. On PHP 8.2 this will typically fail with a visibility error when invoking a private method. Prefer asserting the behavior through the public command output, or explicitly allow reflective access (e.g., via setAccessible(true)) before invoke().

[Copilot]

Pull request overview

Copilot reviewed 30 out of 32 changed files in this pull request and generated 2 comments.

[Copilot]

The JSON fallback emitted when rendering fails a second time includes backslashes (e.g. {\"ok\":false...}), which will produce invalid JSON output. Please output a valid JSON string here (or reuse the same encoder with a minimal payload) so --format json is always parseable even in the fallback path.

Suggested change

	$output->writeln('{"ok":false,"error":"Unable to render enforcement output."}');
	$output->writeln(json_encode([
	'ok' => false,
	'error' => 'Unable to render enforcement output.',
	], JSON_THROW_ON_ERROR));

[Copilot]

escapeCommandValue() is used for both annotation properties and the message. Escaping : and , in the message can cause those characters to be percent-encoded in the final annotation text (GitHub’s runner only requires :/, escaping for the properties segment). Consider splitting this into two functions (escapeData for message: %, \r, \n; escapeProperty for properties: %, \r, \n, :, ,) to avoid over-escaping the annotation message content.

[Copilot]

Pull request overview

Copilot reviewed 30 out of 32 changed files in this pull request and generated 2 comments.

[Copilot]

The YAML generated by decisionYamlWithRules() relies on leading-space interpolation (e.g., {$yamlPaths}decision: / {$yamlRules} allow: []). This becomes invalid YAML when $paths or $forbidRules is empty (it would indent decision:/allow: incorrectly under the previous key). Consider generating these blocks with explicit newlines/indentation (or building an array and dumping via Symfony YAML) so the output is valid regardless of empty inputs and the indentation is stable/readable.

[Copilot]

The example decision scope uses src/Order/*, but scope matching uses fnmatch, so * will not match nested paths (e.g., src/Order/Sub/Foo.php). This is inconsistent with the Semgrep example rule which scans src/Order/**/*.php, and would lead to unmapped findings for nested Order paths. Consider adding a recursive pattern like src/Order/**/* (or narrowing the Semgrep include to src/Order/*.php) to keep the examples and enforcement behavior aligned.

[Copilot]

Pull request overview

Copilot reviewed 38 out of 40 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/Decision/Rules.php:33

• There are blank lines containing trailing whitespace in this file (e.g., around the property declarations and methods). Please remove trailing whitespace to keep diffs clean and avoid style-checker noise.

    /** @var list<string> */
    private readonly array $forbid;

    /** @var list<string> */
    private readonly array $allow;
    
    public function forbid(): array
    {
        return $this->forbid;
    }
    
    public function allow(): array
    {
        return $this->allow;
    }

[Copilot]

Pull request overview

Copilot reviewed 38 out of 40 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 38 out of 40 changed files in this pull request and generated 2 comments.

[Copilot]

$violationsByDecisionId is built in the order findings are processed, so both the text output and the JSON payload ordering will vary with analyzer result ordering. For CI/bot consumption (PR comments, annotations), this can cause noisy diffs between runs. Consider sorting the result deterministically (e.g., ksort() by decision ID, and optionally sorting each decision’s violations by path/line/tool) before returning the EnforcementMatchResult.

[Copilot]

enforce always instantiates YamlDecisionLoader with caching enabled, which will create/update .phpdecide-decisions.cache inside the decisions directory. For CI usage and consistency with explain, consider adding a --no-cache flag (and/or honoring PHPDECIDE_DECISIONS_CACHE) so users can opt out of writing cache files when they want a read-only run.

[Copilot]

Pull request overview

Copilot reviewed 38 out of 40 changed files in this pull request and generated 1 comment.

[Copilot]

Scope::appliesTo() uses fnmatch() against decision path patterns that are documented/expected to use forward slashes. If analyzer reports provide Windows-style paths (e.g. from PHPStan on Windows), \ separators will not match patterns like src/Order/**/*, causing false “unmapped” findings. Normalize AnalyzerFinding::path() (e.g. replace \\ with / and trim leading ./) before calling $repository->applicableTo() and before using the path as a cache key.

[Copilot]

Pull request overview

Copilot reviewed 38 out of 40 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/CLI/DecisionsLintCommand.php:175

• DecisionsLintCommand now always calls DecisionFactory::fromArray() even when validateDecisionArrays() already reported schema errors for the same file. Since Rules now validates/normalizes tokens, this can lead to duplicate error messages (one from validateDecisionArrays, one from the thrown exception) and unnecessary work. Consider short-circuiting: if validateDecisionArrays() returns any errors for a file, skip DecisionFactory::fromArray() for that file.

            $errors = array_merge($errors, $this->validateDecisionArrays($data, $fileLabel));

            try {
                $decision = DecisionFactory::fromArray($data);
                $decisions[] = $decision;
            } catch (InvalidArgumentException $e) {
                $errors[] = sprintf('%s: %s', $fileLabel, $e->getMessage());
                continue;

[Copilot]

Pull request overview

Copilot reviewed 39 out of 41 changed files in this pull request and generated 1 comment.

[Copilot]

This new test method is indented inconsistently with the rest of the file (extra leading spaces on the method declaration and most statements). It still runs, but it violates the file’s established formatting and makes future edits/error diffs harder to read. Reformat the method body and its heredoc call to match the surrounding PSR-12-style indentation.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

Pull request overview

Copilot reviewed 39 out of 41 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 39 out of 41 changed files in this pull request and generated no new comments.