feat(ship-pr): add test evidence gate to Screenshot Check

[thesergsb]

Summary

• Adds a new bullet to /ship-pr's Screenshot Check section that gates on new test files (*.test.*, *.spec.*)
• When the diff includes new test files, the PR body must include verbose test runner output as proof the tests pass
• For Playwright/E2E tests, screenshots of the tested behavior are also required
• STOPs the ship flow if no evidence exists, prompting the user to run tests and capture output

Closes / Addresses

• No existing tickets or TODOs addressed. This is a new enhancement to the ship-pr skill.

Test plan

• Run /ship-pr on a branch that adds new test files — verify it stops and requests test output evidence
• Run /ship-pr on a branch with no new test files — verify no change in behavior
• Verify the new bullet renders correctly in the SKILL.md

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR extends the /ship-pr skill's Screenshot Check with a test evidence gate: when the diff introduces new *.test.* or *.spec.* files, the PR body must include verbose test runner output (and, for Playwright/E2E tests, screenshots) before the ship flow can proceed. The version is bumped to 0.8.1 and the changelog is updated accordingly.

The feature fits naturally into the existing Screenshot Check block and the overall "no unverified changes" philosophy of the skill. However, there are two logic gaps worth addressing before relying on this gate in production:

• No E2E detection heuristic — the instruction requires screenshots "if the tests are Playwright/E2E tests" but provides no criteria for the agent to make that determination, since *.spec.ts is shared by Playwright, Vitest, and Jest.
• No handling for test failures — the gate only stops when evidence is absent; if the developer runs the tests and they fail, the current wording would have the agent embed failure output and continue, rather than blocking until the tests are green.
• JS/TS-only file patterns — *.test.* / *.spec.* miss Python (test_*.py), Go (*_test.go), and Ruby (*_spec.rb) conventions, which could leave test additions in polyglot repos ungated.

Confidence Score: 3/5

• Safe to merge for JS/TS-only repos, but two logic gaps in the gate's instructions could allow failing tests or ungated Playwright changes to slip through.
• The change is small and well-scoped, but the missing E2E detection heuristic and lack of guidance for failing (vs. absent) test evidence are functional gaps in the gate's logic that could reduce its effectiveness in practice.
• skills/ship-pr/SKILL.md — the new test evidence gate bullet needs clarification on Playwright detection and failure handling.

Important Files Changed

Filename	Overview
skills/ship-pr/SKILL.md	Adds a test evidence gate to the Screenshot Check section. Two logic gaps: no heuristic to distinguish Playwright/E2E tests from unit tests, and no explicit handling when run tests fail rather than merely being absent.
CHANGELOG.md	Adds a 0.8.1 changelog entry accurately describing the new test evidence gate feature. No issues found.
VERSION	Bumps version from 0.8.0 to 0.8.1, consistent with a patch-level addition. No issues found.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Step 3: PR Creation] --> B[Screenshot Check]
    B --> C{Diff includes UI/frontend files?}
    C -- Yes --> D{evidence/screenshots/ empty or missing?}
    D -- Yes --> E[STOP: warn user, recommend /validate]
    D -- No --> F{Diff adds new test files?\n*.test.* / *.spec.*}
    C -- No --> F
    F -- Yes --> G{PR body includes test runner output?}
    G -- No --> H[STOP: run tests now and capture output]
    G -- Yes --> I{Tests are Playwright/E2E?\n⚠️ no detection heuristic defined}
    I -- Yes --> J{Screenshots of tested behavior embedded?}
    J -- No --> H
    J -- Yes --> K{Screenshots exist in evidence/?}
    I -- No --> K
    F -- No --> K
    K -- Yes --> L[Embed screenshots in PR body\nusing absolute GitHub blob URLs]
    K -- No --> M[Continue without screenshots]
    L --> N[Commit + Push + Create/Update PR]
    M --> N

    style E fill:#f66,color:#fff
    style H fill:#f66,color:#fff
    style I fill:#fa0,color:#000

Loading

_{Reviews (1): Last reviewed commit: "chore: bump to v0.8.1" | Re-trigger Greptile}

[greptile-apps]

Test file patterns are JS/TS-only

The patterns *.test.* and *.spec.* only cover JavaScript/TypeScript naming conventions. Common test file patterns in other ecosystems would be silently skipped:

• Python: test_*.py / *_test.py
• Go: *_test.go
• Ruby: *_spec.rb / test_*.rb
• Rust: inline test modules won't match file-level patterns at all

If this project's skills are ever used on polyglot repos, the gate would never fire for non-JS test additions. Consider broadening the detection heuristic or noting the limitation explicitly.

Suggested change

	- If the diff adds new test files (*.test.*, *.spec.*): the PR body MUST include
	proof that the new tests pass. At minimum, embed the verbose test runner output.
	If the tests are Playwright/E2E tests, also capture and embed screenshots of the
	tested behavior. STOP if no evidence exists — run the tests now and capture output.
	- If the diff adds new test files (`*.test.*`, `*.spec.*`, `test_*.py`, `*_test.py`,
	`*_test.go`, `*_spec.rb`, `test_*.rb`): the PR body MUST include
	proof that the new tests pass. At minimum, embed the verbose test runner output.
	If the tests are Playwright/E2E tests, also capture and embed screenshots of the
	tested behavior. STOP if no evidence exists — run the tests now and capture output.

[greptile-apps]

No guidance on how to identify Playwright/E2E tests

The instruction branches on "If the tests are Playwright/E2E tests" but gives no criteria for making that determination. A *.spec.ts file could be a Playwright test, a Vitest unit test, or a Jest integration test — they all use the same naming convention.

Without disambiguation rules, the AI agent has to guess, which could either:

• Over-require screenshots (blocking the flow unnecessarily for unit tests)
• Under-require them (missing screenshots for real E2E tests)

Consider adding detection heuristics, for example: check for playwright imports, look for a playwright.config.* at the repo root, or check the directory (e.g. e2e/, tests/e2e/).

[greptile-apps]

No handling for test failures

The gate says "STOP if no evidence exists — run the tests now and capture output," which addresses missing evidence but not the case where tests are run and fail. If the developer runs the tests and they fail, the current instruction still asks them to "capture output" — which would embed failure output into the PR body.

Consider adding an explicit instruction: if the tests fail, the ship flow should stop and the developer must fix the failures before proceeding, rather than just capturing the output of a failing run.