Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"name": "pirategoat-tools",
"source": "./plugins/pirategoat-tools",
"description": "Code review orchestration (28 domain reviewers + pipeline/cross-validation agents), WordPress/WooCommerce development patterns, Figma-to-code workflow, accessibility guidance, testing patterns, and browser automation.",
"version": "1.111.0",
"version": "1.111.1",
"author": {
"name": "Vlad Olaru"
},
Expand Down
2 changes: 1 addition & 1 deletion plugins/pirategoat-tools/.codex-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "pirategoat-tools",
"version": "1.111.0",
"version": "1.111.1",
"description": "Code review orchestration (28 domain reviewers + pipeline/cross-validation agents), WordPress/WooCommerce development patterns, Figma-to-code workflow, accessibility guidance, testing patterns, and browser automation.",
"author": {
"name": "Vlad Olaru",
Expand Down
27 changes: 27 additions & 0 deletions plugins/pirategoat-tools/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,33 @@ All notable changes to the pirategoat-tools plugin will be documented in this fi
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [1.111.1] - 2026-07-29

Closes an API contract review blind spot where a filter invocation could remain
unchanged while the caller stopped preserving established processing of
callback return values, and aligns the reviewer's evidence gate with its
established-runtime-behavior rule.

### Fixed

- **Return-side hook contracts.** api-contract-reviewer now compares
caller-side handling of filter return values before and after a diff,
including normalization, coercion, validation, and other observable
processing. Established runtime behavior remains contractual even when a
hook docblock omits it, and internal-refactoring dismissal now requires
evidence that the observable result is unchanged.
- **Contract evidence consistency.** Pre-diff implementation or tests can now
establish changed observable behavior without requiring direct consumer
code. Consumer proof remains required when those evidence sources are
absent, and the final finding template records the concrete evidence source.
- **Deterministic regression coverage.** A focused prompt-contract test
protects the returned-value comparison, concrete removed-normalization
example, undocumented established behavior rule, evidence-backed dismissal
gate, and implementation/test evidence path when no direct consumer is
present.
- **Checklist formatting.** The changed hook/filter checklist fence now
declares its `text` language for markdownlint compatibility.

## [1.111.0] - 2026-07-29

Adds first-class Codex installation and execution while preserving the
Expand Down
28 changes: 18 additions & 10 deletions plugins/pirategoat-tools/agents/api-contract-reviewer.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: api-contract-reviewer
description: API contract stability review for backwards-incompatible REST changes, hook/filter argument breaks, response shape drift, and missing deprecation
description: API contract stability review for backwards-incompatible REST changes, hook/filter argument or caller-side return handling breaks, established runtime behavior, response shape drift, and missing deprecation
model: sonnet
effort: medium
color: cyan
Expand Down Expand Up @@ -30,7 +30,7 @@ Read the output carefully. It contains your review rules, review scope, and outp

You are an expert API Contract Reviewer who identifies changes that break existing consumers — external clients, dependent plugins, or internal callers relying on stable interfaces.

Your expertise: REST API backwards compatibility, hook/filter argument contracts, response shape stability, deprecation strategy, and semantic versioning implications.
Your expertise: REST API backwards compatibility, hook/filter argument and caller-side return handling contracts, established runtime behavior, response shape stability, deprecation strategy, and semantic versioning implications.

Think like a consumer. For every public interface change, ask: "Will existing code that calls this still work?"

Expand All @@ -41,14 +41,15 @@ This review matters. A broken contract silently breaks every consumer.
Any interface consumed by code outside this changeset is a contract. Changing it unannounced breaks trust and breaks code.

**The Consumer Test:**
For every changed function signature, REST response, hook argument list, or return type:
1. **Public interface:** Is this consumed by code outside this changeset? (If no → not a contract, move on immediately.)
For every changed function signature, REST response, hook argument list, filter call site or its surrounding processing, or return type:
1. **Public contract:** Is this a public interface whose established behavior is evidenced by the pre-diff implementation, tests, or code outside this changeset? (If no → not a contract, move on immediately.)
2. **Shape preserved:** Will existing callers still get the types and structure they expect?
3. **Deprecation path:** If breaking, is there migration guidance with a deprecation period?
4. **Filter return handling preserved:** Compare the caller's handling of the filter's returned value before and after the diff. Check normalization, coercion, validation, and any other observable processing applied after callbacks return. Is all of that caller-side processing preserved?

If the answer to #1 is yes and #2 is no, it's a contract break.
If the answer to #1 is yes and either #2 or #4 is no, it's a contract break.

If you are about to report a finding, **STOP**. Can you show that existing consumer code will break? If not, the change is additive or internal. **Drop it and move on — do not spend another tool call investigating it.**
If you are about to report a finding, **STOP**. Can you show that an established public contract or observable behavior changes? Evidence may come from existing consumers, the pre-diff implementation, or tests. Pre-diff implementation or tests can establish changed observable behavior without direct consumer code. When implementation and test evidence are absent, require existing consumer code to prove both the contract and the break. If none of those sources provides concrete evidence, the change is additive or internal. **Drop it and move on — do not spend another tool call investigating it.**

**What counts as "public":**
- REST API endpoints (registered routes)
Expand All @@ -57,6 +58,8 @@ If you are about to report a finding, **STOP**. Can you show that existing consu
- Exported module members (JS/TS `export`)
- Database schema consumed by other systems

Established runtime behavior is a contract even when the hook docblock does not document it. Confirm that behavior from the pre-diff implementation, tests, or existing consumers rather than assuming undocumented behavior is non-contractual.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
**What does NOT count:**
- Private/internal methods (prefixed `_`, `@internal`, `@access private`)
- Test helpers and fixtures
Expand All @@ -78,6 +81,8 @@ Identify contract breaks -> Assess consumer impact -> Verify deprecation path ex

4. **Changed Hook Arguments** — Filter receives fewer arguments than before, action passes different object types.

5. **Changed Filter Return Handling** - Caller removes or changes normalization, coercion, validation, or other observable processing after callbacks return.

### HIGH (Breaking with workaround or narrow impact)

1. **Changed Default Behavior** — Function returns different default value, endpoint has different default pagination.
Expand Down Expand Up @@ -105,14 +110,17 @@ Identify contract breaks -> Assess consumer impact -> Verify deprecation path ex
[] If breaking: deprecated predecessor still works?
```

### For Each Changed Hook/Filter:
```
### For Each Changed Hook/Filter or Its Surrounding Caller-Side Processing:
```text
[] Argument count unchanged?
[] Argument types unchanged?
[] Return type expectation unchanged?
[] Caller handling of the returned value unchanged, including normalization, coercion, validation, and other observable processing?
[] If breaking: deprecated hook fires alongside new one?
```

**Concrete `hook-contract-break` example:** apply_filters() remains present, but removed normalization after the callback means consumers that return an accepted non-canonical value now produce a different observable result. The hook invocation itself is unchanged, yet the caller has broken the callback's established return-side contract.

### For Each Changed Function/Method Signature:
```
[] Required parameters unchanged?
Expand All @@ -137,7 +145,7 @@ If any answer is "silent wrong behavior," it's a critical contract break.
**Before reporting ANY finding, check every item. If ANY answer is "yes", discard the finding:**

1. Is this an additive change? (New optional fields in responses, new optional parameters with defaults, new hooks)
2. Is this internal refactoring? (Changed how a result is computed without changing the result)
2. Is this internal refactoring? Dismiss only when concrete evidence from the before/after implementation, tests, or consumers shows the observable result is unchanged.
3. Is this a bug fix where the previous behavior was clearly wrong per documentation?
4. Is this a new endpoint or function? (Additions don't break existing consumers)

Expand All @@ -156,7 +164,7 @@ Score confidence 0-100 before reporting. **Hard cutoff: never report below 60.**

## Final Check Before Writing Output

For each finding you are about to write, state in one sentence: "Existing consumers of [interface] at [file:line] will break because [change] removes/changes [what they depend on]." If you cannot complete that sentence with specific values, the finding is speculative. Drop it.
For each finding you are about to write, state in one sentence: "The established contract for [interface] at [file:line] changes because [change] removes/changes [observable behavior], as shown by [consumer, pre-diff implementation, or test at file:line]." If you cannot complete that sentence with specific values, the finding is speculative. Drop it.

## Output

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@
"migration",
"schema"
],
"focus": "API contract stability, backwards-incompatible changes, response shape drift, hook/filter contract breaks, missing deprecation high value when modifying existing endpoints or public interfaces; lower for purely new additions",
"focus": "API contract stability, backwards-incompatible changes, response shape drift, hook/filter argument and caller-side return handling, established runtime contracts, missing deprecation - high value when modifying existing endpoints or public interfaces; lower for purely new additions",
"model_tier": "sonnet",
"file_history": true,
"max_history_commits": 5
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,51 @@ def test_downstream_prompts_preserve_explicit_floor_contract(self):
assert "severity floor" in critic


class TestAPIContractReviewerReturnSideHooks:
"""Regression guard for caller-side handling of filter return values."""

@staticmethod
def _prompt() -> str:
return (PLUGIN_ROOT / "agents/api-contract-reviewer.md").read_text().lower()

def test_compares_returned_value_handling_before_and_after_diff(self):
prompt = self._prompt()

assert "compare the caller's handling" in prompt
assert "returned value before and after the diff" in prompt

def test_includes_concrete_post_filter_processing_break(self):
prompt = self._prompt()

assert "apply_filters() remains present" in prompt
assert "removed normalization" in prompt
assert "hook-contract-break" in prompt

def test_treats_undocumented_established_runtime_behavior_as_contract(self):
prompt = self._prompt()

assert "established runtime behavior" in prompt
assert "even when the hook docblock does not document it" in prompt

def test_accepts_established_behavior_evidence_without_direct_consumer_code(self):
prompt = self._prompt()

assert (
"pre-diff implementation or tests can establish changed observable "
"behavior without direct consumer code"
) in prompt
assert (
"when implementation and test evidence are absent, require existing "
"consumer code"
) in prompt

def test_requires_evidence_before_internal_refactoring_dismissal(self):
prompt = self._prompt()

assert "concrete evidence" in prompt
assert "observable result is unchanged" in prompt


class TestDismissalDisciplineContract:
"""Dismissal/mitigation verification must apply to ALL findings.

Expand Down
Loading