Merge pull request #757 from craigk5n/fix/concurrent-test-isolation Give every test run its own ports and paths

fix: require the test fixture rather than relying on the bootstrap The previous commit loaded tests/McpServerFixture.php from tests/bootstrap.php. That passed locally and produced 57 “Class McpServerFixture not found” errors in CI, because .github/workflows/test-mcp.yml runs vendor/bin/phpunit tests/McpTest.php with no -c, so phpunit.xml is never read and its bootstrap never loads. tests/run_unit_tests.sh invokes five more files the same way. This was self-inflicted twice over. The first attempt added a require to each test class and reached only those whose existing requires used the same quoting style, so I moved the load into the bootstrap and called it cleaner. It traded a drift problem for a dependency on how phpunit is invoked, which is worse because it is invisible until a workflow that invokes it differently runs. Running only `phpunit -c tests/phpunit.xml` locally could never have caught it. Every file that uses the fixture now requires it, which is the convention in this tree and does not care how phpunit was started. Verified against both invocations: `phpunit tests/McpTest.php` and `phpunit -c tests/phpunit.xml`. tests/TestFixtureRequiresTest.php fails when any test uses McpServerFixture, McpTestHelper or CrossDatabaseTestHelper without requiring it, and says why in the message. Checked by deleting one require. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

test: give every run its own ports and paths Six MCP test classes hard-coded a port between 8099 and 8104, a SQLite file and a server log path. McpTestHelper and CrossDatabaseTestHelper defaulted to fixed database paths, and two probes wrote scratch scripts straight into tests/ under fixed names. All of that is machine-wide, so two runs deleted each other’s files and competed for the ports, and the damage showed up as errors in tests that had nothing to do with it. Ports now come from the kernel: bind to port 0, read back what was assigned, release it. There is a window between releasing and `php -S` taking it, which is unavoidable without passing the socket itself, and the callers already confirm the server answers before running anything. Measured rather than assumed. Two concurrent runs of the MCP and cross-database tests, with the lock disabled: before 59 errors and 57 errors after ports and databases 1 error and 1 error after all six collisions clean, and clean again at three runs Two concurrent full suites now pass: 792 tests each, no errors. The last two collisions were the ones reading the code would not have found: tests/test_bearer_bug.php and tests/test_duplicate_bug.php are written into the repository directory, so whichever run finished first deleted the file the other was still executing. tests/McpServerFixture.php holds both helpers and is loaded from tests/bootstrap.php rather than by each test file. The first attempt added a require to every class and silently reached only those whose existing requires used the same quoting style, which is the kind of thing that leaves half a change working. The lock in tests/bootstrap.php is left in place. The collisions it guards are gone, but one new test with a fixed path would bring them back, and the failure is expensive to read. AGENT_ROADMAP.md records what removing it would need. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Merge pull request #756 from craigk5n/feat/ai-signal-lint Flag machine-written comments on lines a change adds

fix: let a comment opt out of the AI-signal check The new job failed on its first run in CI, on this file’s own docblock: tools/check-ai-signals.php:29 (“You can test this script from the command line”), which is good writing That line quotes an example of the phrasing the tool detects, inside the explanation of why the tool does not flag such comments in existing code. It is a real category rather than a one-off: anything discussing these phrases has to be able to quote one, including this tool, its test, and any comment explaining why a particular wording was chosen. A comment containing ai-signals:allow is now skipped. It is deliberately a visible marker in the comment rather than a configuration file, so a reader sees that the author made a decision instead of wondering why the check did not apply. It covers only the comment it appears in, so it cannot be dropped at the top of a file to switch the check off for everything below. There is a test for that, and writing it found that the first draft of the suppression test used two separate // lines, which PHP tokenises as two comments — the marker correctly did not reach the second. The fixture now uses one docblock, which is how the marker is actually used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>