bug-verify

name

bug-verify

description

Final step of the php-src bug-fix flow. Runs the related test suite, checks for new compiler warnings, optionally runs the full test suite or AddressSanitizer/Valgrind, and flips SPEC.md status to done. Use when the user says "verify the fix", "/bug-verify", "did I break anything", "run the regression checks", "check the fix", or otherwise wants to confirm the fix is safe to commit. Refuses if Implementation phase is not done.

bug-verify

Fourth and last step of the bug-fix flow. Owns the Verification phase. Confirms the fix doesn't regress nearby tests and doesn't introduce new compiler warnings.

Inputs

1. SPEC.md in the task directory resolved per _shared/task-dir.md (env var → .current → cwd), type: bug, Implementation phase done.
2. $PHP_SRC_DIR with the post-fix sapi/cli/php built.

What it produces

• Updated SPEC.md: Verification phase done (or blocked with reasons), top-level status: done.
• A summary suitable for the user's commit / PR description.

Phase owned

Verification.

Steps this skill adds (some are optional):

• related-tests — make TESTS=<related-dir> test or run-tests.php <related-dir> passes.
• dependents-tests — extensions that link against (or #include headers from) the changed translation unit pass their own test directories. Always added by default; one sub-step per dependent extension that triggers any new failure. Catches the cross-extension regression case where e.g. a change in ext/date breaks ext/spl.
• warnings-clean — no new compiler warnings versus php_src_ref (the SHA recorded in SPEC.md frontmatter).
• full-test (optional) — make test; only added if the user explicitly asks.
• valgrind / asan (optional) — only for memory-sensitive fixes; only on user request.

Workflow

1. Pre-guardrails

• SPEC.md type: bug, Implementation phase done. Refuse otherwise.
• The new .phpt from Reproduction is recorded.

2. Choose the related test directory

From the Implementation steps (impl-N) and the Reproduction test path, pick the nearest meaningful test directory:

Changed file	Related test dir
ext/<name>/...	ext/<name>/tests/
Zend/...	Zend/tests/
main/streams/...	tests/streams/
sapi/<name>/...	sapi/<name>/tests/

If the change touches multiple subsystems, run each. Add a related-tests step per directory.

3. Run related tests

cd "$PHP_SRC_DIR"
sapi/cli/php run-tests.php -j$(nproc) <related-dir>

Capture:

• total passed / failed / skipped / borked
• names of any newly failing tests

Newly failing tests = tests that pass on the pre-fix ref but fail on the post-fix build. Use the php_src_ref SHA from SPEC.md frontmatter to compare if needed:

git -C "$PHP_SRC_DIR" stash push -u -m "bug-verify-baseline-check"
git -C "$PHP_SRC_DIR" checkout <php_src_ref>
make -j$(nproc) >/dev/null
sapi/cli/php run-tests.php -j$(nproc) <related-dir> > /tmp/before.txt
git -C "$PHP_SRC_DIR" checkout -
git -C "$PHP_SRC_DIR" stash pop
make -j$(nproc) >/dev/null
sapi/cli/php run-tests.php -j$(nproc) <related-dir> > /tmp/after.txt
diff /tmp/before.txt /tmp/after.txt

Only do the baseline rebuild if there is at least one new failure — it's slow. Otherwise: zero new failures = related-tests step done.

Any new failure that isn't the bug fix's expected .phpt flip is a regression candidate. Mark related-tests blocked with the failing test names and .diff excerpts. Don't keep going.

3b. Run dependents' tests (broad scope, default)

PHP's test files are not perfectly partitioned by extension — a change in ext/date can break ext/spl (or vice versa) through shared headers, registered handlers, or runtime callbacks. Default scope is broad: in addition to the related directory, run the test directories of extensions that depend on the changed translation unit.

Compute the dependent set from the changed files:

1. For each header touched in Implementation (ext/<name>/*.h, Zend/*.h, main/*.h), grep all other extensions for #include "<header>" or #include <header>:

   grep -rln --include='*.[ch]' --include='config.m4' "<header-basename>" "$PHP_SRC_DIR/ext" "$PHP_SRC_DIR/sapi" \
     | xargs -I{} dirname {} | sort -u
   

2. For each touched .c whose symbols are exported (function declared in a header), grep for callers across extensions:

   grep -rln --include='*.c' "<exported-symbol>" "$PHP_SRC_DIR/ext" "$PHP_SRC_DIR/sapi" \
     | xargs -I{} dirname {} | sort -u
   

3. Map each unique directory back to its extension's tests/ directory.
4. Subtract the already-covered related-tests directory.

Then run:

cd "$PHP_SRC_DIR"
for d in "${DEPENDENT_TEST_DIRS[@]}"; do
  sapi/cli/php run-tests.php -j$(nproc) "$d"
done

For each dependent extension X whose tests show any new failure (compared to the php_src_ref baseline — re-use the diff approach from § 3), add a dependents-tests:X sub-step. Mark done if zero new failures; mark blocked with the failing test names otherwise.

If the dependent set is empty (the change touches a private .c with no header exports, or a header included by no other extension), mark dependents-tests done with a one-line note no dependents under static analysis.

4. Warning check

cd "$PHP_SRC_DIR"
make -j$(nproc) 2>&1 | grep -E '^[^:]+:[0-9]+:[0-9]+: warning:' | sort -u > /tmp/warnings.after

Compare to a baseline. If a baseline wasn't captured before the fix, rebuild the pre-fix SHA in a side tree:

git -C "$PHP_SRC_DIR" stash push -u -m "bug-verify-warnings-baseline"
git -C "$PHP_SRC_DIR" checkout <php_src_ref>
make -j$(nproc) 2>&1 | grep -E '^[^:]+:[0-9]+:[0-9]+: warning:' | sort -u > /tmp/warnings.before
git -C "$PHP_SRC_DIR" checkout -
git -C "$PHP_SRC_DIR" stash pop
make -j$(nproc) >/dev/null
diff /tmp/warnings.before /tmp/warnings.after

New warnings introduced by the fix → warnings-clean step blocked with the diff. The user must accept or address them.

If no new warnings: warnings-clean done.

5. Optional extras (on user request only)

If the user asked for full suite or memory-safety:

• Full test: make TEST_PHP_ARGS="-j$(nproc)" test. Add a full-test step. Long-running; record the duration.
• AddressSanitizer: confirm the build was configured with --enable-address-sanitizer. If not, output the configure command and stop without adding the step.
• Valgrind: sapi/cli/php run-tests.php -m <test>. Use sparingly — very slow.

Don't add these proactively. They're opt-in.

6. Finalise

• All required steps green → Verification phase done, SPEC.md top-level status: done.
• Any step blocked → Verification phase stays in-progress; SPEC.md top-level status: blocked with the failing step pointed to.

7. Report

Verification complete: <PASS | BLOCKED>
- related-tests:    <pass/fail breakdown>
- dependents-tests: <N extensions checked; M new failures>
- warnings-clean:   <no new warnings | N new warnings>
- full-test:        <skipped | passed | N failures>

SPEC.md status: <done | blocked>

If done, suggested commit message is in the Implementation phase report (see SPEC.md).

When SPEC.md status: done and target_branch is not master, append one hint line:

Fix landed on <target_branch>. To propagate to higher maintained branches:
  /upmerge

Never run /upmerge automatically — the user decides when to upmerge.

Boundaries

• No code changes. Read-only against the source tree (except for transient stash/checkout which is restored at the end). If related-tests reveals a regression, the user re-runs /bug-fix to address it — this skill doesn't loop back.
• No git commit / git push. The user owns the remote.
• Don't auto-run full test suite or sanitizers. They take long enough to deserve explicit consent.

Edge cases

Situation	Handling
Related tests pass but a sibling extension breaks	Caught automatically by dependents-tests (§ 3b). If the dependent extension wasn't discovered by the header/symbol grep (e.g. dynamic registration via zend_register_* at runtime), add an extra dependents-tests:X sub-step manually and re-run.
The bug's .phpt is in Zend/tests but the fix touched ext/standard	Run both directories — pick the nearest test dir to each touched subsystem.
New warning is on a line the fix added	Block. Either silence it correctly (cast, init) or accept as a known issue — user's call.
Baseline rebuild for warnings is slow	Note the time in the step body; mention to user it's a one-time cost.
User asks for make test (full suite) on a slow machine	Note expected runtime (often 5–15 min); proceed only if user confirms.
Stash/checkout fails due to dirty tree the skill itself can't clean	Stop with Working tree dirty in $PHP_SRC_DIR; commit or stash before verifying.
Test runner reports BORK	Treat as blocked — environment issue, surface to user.

Quality checklist

• related-tests ran against at least one directory matching the changed file paths
• dependents-tests ran against every extension that grep-discovers a header/symbol dependency on the changed code (or recorded no dependents under static analysis)
• No newly-failing tests in either related or dependent directories (other than the bug's .phpt which now passes)
• warnings-clean step has a concrete answer (compared against php_src_ref)
• Optional full-test / valgrind / asan steps are present only if the user asked
• SPEC.md top-level status flipped to done or blocked with reason
• When status: done and target_branch != master, the final report hints at /upmerge
• Progress Log appended

Out of scope

• Performance benchmarking — not part of the bug flow. /idea-prototype covers perf hypotheses.
• Posting results to the GH issue — never auto.
• Re-running /bug-fix after a regression is detected — surfaced for the user to decide.