Pin default-terminal handler absence per signal kind

[jeremy]

Absorbs the one #645 review finding that merged unaddressed (the resolved-without-reply P2 on conformance/event-feed/schema.json, thread): the default-terminal pin checked whether config.signalDisposition was present at all, not whether the disposition for the signal that actually terminated the feed was configured. A buffer_overflow terminal configuring only a feedGap key therefore escaped the empty-invocations requirement and could claim a phantom handler invocation — the pin went vacuous exactly when a cross-signal scenario would need it.

Change

allOf[1] (one whole-object pin) becomes two per-signal pins. Each applies when finally.error.reason is const buffer_overflow / feed_gap and config.signalDisposition lacks that signal's own key (absent entirely, or present without the key — the not-wrapped composition covers both), and then requires finally.handlerInvocations whose exact contains zero invocations of that kind (a not/contains on kind, per commit 2: const [] said more than the contract does — it would have rejected a legitimate future cross-signal run such as an accepted overflow followed by an unhandled feed_gap, which Copilot's suppressed comment correctly flagged). The configured⇒record-required pin and the forbidden-side pin are unchanged.

Proof

Probe: a fixture-21 clone with signalDisposition: {"feedGap": "accept"} and a phantom {bufferOverflow, accept} invocation; mirror probe swaps the roles (feed_gap terminal, bufferOverflow-only key, phantom {feedGap, accept}).

A third probe, the valid cross-signal control, is the legitimate shape of the same scenario: bufferOverflow handler configured and invoked (accept), run ends feed_gap default-terminal, record [{bufferOverflow, accept}].

Leg	Schema	Expectation	Real exit
A	old (main, ee28e96c…)	probe accepted — the escape exists	A_REAL_EXIT=0
B	commit 1 (const [] form)	probe rejected at the pin	B_REAL_EXIT=1
C	old	mirror probe accepted	C_REAL_EXIT=0
D	commit 1	mirror probe rejected	D_REAL_EXIT=1
E	final (acebead2…, per-kind form)	probe still rejected (should not be valid under contains kind: bufferOverflow)	E_REAL_EXIT=1
F	final	mirror probe still rejected	F_REAL_EXIT=1
G	final	valid cross-signal control accepted	G_REAL_EXIT=0
H	commit 1	valid cross-signal control rejected — the over-strictness the per-kind form removes	H_REAL_EXIT=1

make event-feed-fixtures-check green on this head (metaschema + all 22 fixtures), GATE_REAL_EXIT=0. No scenario-fixture changes: today's suite has no cross-signal scenario, so every existing fixture satisfies the tighter pins as-is — the change only closes the door PR 4's future fixtures could otherwise walk through.

Committed regression gate (commits 3–5): pin probes with derived mutants

Copilot's second review round made a fair point: the probes above were PR-body evidence only, so a later schema edit (PR 4, PR-T) could quietly re-widen the pins with CI staying green. The first committed shape (invalid fixtures asserted rejected, then + paired positive controls) drew two more correct findings — any nonzero validator exit counted as rejection (a malformed file would false-green the gate), and basename pairing couldn't enforce the minimal delta between fixture and control. Rather than patch the artifact a third time, commit 5 removes it: the invalid case is derived, not committed.

Each conformance/event-feed/pin-probes/*.json declares a control scenario and one mutation (path + value). scripts/check-event-feed-pin-probes.py (run by make event-feed-fixtures-check) validates the control — must pass — then applies the mutation in-process and validates the mutant — must fail. The isolation claim is now true by construction: the accepted/rejected delta is exactly the declared mutation; a malformed probe is a hard parse error, never a "rejection"; the mutant derives from a control that just parsed and validated, so it cannot be malformed; the control-first ordering means a nonzero on the mutant can only be a schema rejection; and a mutation that misses the control or equals its value fails as vacuous.

Round 5 (commit 6) tightened two things inside that shape, both from review findings: each control now records the legal other-kind invocation (so the probes guard both directions — a regression back to the over-strict const [] fails at the control check, and a pin removal fails at the mutant check), and the mutant's rejection must prove itself via the validator's structured JSON output (status: fail, non-empty errors, empty parse_errors) — a transient tool failure or schema-parse error on that invocation fails the gate instead of printing pin-verified.

Test-the-test, legs L–V across the rounds: gate green on the committed schema; the pre-fix (pin-less) schema fails at the mutant check; the over-strict const [] schema fails at the control check; an unrelated always-reject schema fails at the control check; a malformed probe, a typo'd mutation path, and a vacuous mutation each fail with their own error; and the rejection discriminator's unit cases (tool-error text, parse_errors, empty errors, success, genuine rejection) all behave. The mechanism is closed-form — further hardening rounds get triaged against whether they name a failure derivation-plus-structured-rejection misses.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the event-feed conformance JSON schema to pin “default-terminal” expectations per signal (buffer_overflow vs feed_gap) instead of treating them as a combined case.

Changes:

• Split the previous combined default-terminal rule into two per-signal rules (buffer_overflow and feed_gap).
• Tighten the “no disposition configured” condition to mean “signalDisposition exists but the specific signal key is absent” (or signalDisposition missing).
• Replace enum reason matching with per-rule const reason matching.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

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

Suppressed comments (2)

conformance/event-feed/schema.json:155

• This feed-gap branch similarly rejects a valid run that invokes an accepting bufferOverflow handler before later terminating on an unhandled feed gap, because the shared exact invocation record would correctly be non-empty. The per-signal pin should prohibit only feedGap invocation entries rather than forcing all signal-handler history to be empty.

                    "const": "feed_gap"

conformance/event-feed/schema.json:115

• This over-constrains mixed-signal scenarios. handlerInvocations.exact is the ordered record for both signal kinds, so a valid run could first invoke an accepting feedGap handler and later terminate on an unhandled buffer overflow. Requiring the entire record to equal [] rejects that run even though no overflow handler was invoked. Keep the record required, but forbid only entries whose kind is bufferOverflow.

This issue also appears on line 155 of the same file.

                  "exact": {
                    "const": []
                  }

[Copilot]

Pull request overview

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

Suppressed comments (2)

conformance/event-feed/schema.json:152

• The mirrored feedGap key-absence branch also lacks an automated regression case; fixture 23 omits signalDisposition entirely and therefore does not distinguish this condition from the old one. Please gate a negative case with only bufferOverflow configured and a phantom feedGap invocation so both per-signal pins are continuously verified.

            "not": {
              "required": [
                "signalDisposition"
              ],
              "properties": {
                "signalDisposition": {
                  "required": [
                    "feedGap"
                  ]
                }
              }

conformance/event-feed/schema.json:82

• The new key-specific bufferOverflow absence branch is not exercised by the committed suite: fixture 21 has no signalDisposition at all, so it would also pass with the previous whole-object condition. Please add an automated negative regression case with only feedGap configured and a phantom bufferOverflow invocation; the current manual probe will not protect this fix from regression.

This issue also appears on line 142 of the same file.

            "not": {
              "required": [
                "signalDisposition"
              ],
              "properties": {
                "signalDisposition": {
                  "required": [
                    "bufferOverflow"
                  ]
                }
              }

[Copilot]

Pull request overview

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

Suppressed comments (1)

Makefile:569

• This accepts every nonzero validator exit as proof that the intended per-signal pin fired. A malformed fixture—or a future, unrelated schema constraint that rejects it—would keep this gate green even if the per-signal pin were removed, so the negative fixtures do not actually isolate the regression as the README claims. Require a corresponding positive control (for example, the same fixture with only the phantom invocation removed) to validate before asserting that its negative twin fails.

		if uvx --from 'check-jsonschema==$(CHECK_JSONSCHEMA_VERSION)' check-jsonschema \
			--schemafile conformance/event-feed/schema.json "$$f" >/dev/null 2>&1; then \
			echo "ERROR: $$f validated, but this directory holds shapes the schema must reject"; \
			exit 1; \
		else \
			echo "rejected (as required): $$f"; \

[Copilot]

Pull request overview

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

Suppressed comments (1)

Makefile:565

• Pairing files by basename does not enforce the claimed minimal delta, so control acceptance alone cannot isolate rejection to the pin. The committed pairs already differ in name and description as well as the invocation record, and future edits could add another invalid-only constraint while this gate still passes after the per-signal pin is removed. Make each pair identical outside a declared mutation and have the gate verify that relationship (or derive the invalid case from the control) before using it as proof.

	@for f in conformance/event-feed/invalid-fixtures/*.json; do \
		c="conformance/event-feed/invalid-fixtures/controls/$$(basename $$f)"; \
		test -f "$$c" || { \

[Copilot]

Pull request overview

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

Suppressed comments (1)

conformance/event-feed/pin-probes/phantom-gap-invocation.json:124

• The permissive half of the new per-kind rule is not regression-tested: both committed controls use exact: [], so restoring the earlier over-strict const: [] still accepts both controls and rejects both mutants. This probe already configures and emits bufferOverflow; record {bufferOverflow, accept} in the control and retain it while adding the phantom {feedGap, accept} in the mutant. That also makes the control consistent with the registered-handler contract and ensures an over-strict schema fails at the control check.

      "handlerInvocations": {
        "exact": []

[Copilot]

Pull request overview

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