From 72eb5a100c8253f985a4bc0e3ebdf80cd4092b47 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Mon, 28 Sep 2026 15:10:11 -0600 Subject: [PATCH] feat: apply whetstone retry handler fixes --- assets/agents/adversary/graph.yaml | 18 +++++++++++++++++- assets/skills/code-review/SKILL.md | 9 +++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/assets/agents/adversary/graph.yaml b/assets/agents/adversary/graph.yaml index b06811b..b269829 100644 --- a/assets/agents/adversary/graph.yaml +++ b/assets/agents/adversary/graph.yaml @@ -208,6 +208,19 @@ nodes: values shaped to the criterion's example, TODO/stub presented as done — those are PARTIAL or DIVERGED with the gaming named in the complaint. + Retry / redelivery / idempotency demands: before you mark a criterion PARTIAL + or UNMET for lacking a retry, re-run, or "second attempt emits nothing" spec, + classify the handler. STATEFUL — it carries something across attempts (emits + events, calls an external system, performs an unconditional write, keeps an + in-memory or ledger record) — a retry spec is a legitimate demand. STATELESS — + each attempt is a pure function of the request plus a conditional store + predicate (`WHERE status = 'pending'`, `ended_at IS NULL`, `INSERT … ON + CONFLICT DO NOTHING`) and nothing is emitted or ledgered outside that + predicate — then "retry produces no event" is a tautology of the mechanism, + not a missing behavior: demand at most one test of the predicate itself, + never a retry spec. Name the classification and its file:line evidence in + the complaint either way. + Execution criteria ("tests pass", "lint clean", "build succeeds") are judged ONLY against the recorded verification runs in your prompt — you have no execution tools BY DESIGN and must never claim to have run @@ -297,7 +310,10 @@ nodes: The checklist (each hit becomes one complaint with kind, file:line location, the violation, and the fix): - **Absence** — something the PLAN requires that maps to no criterion and - never showed up in the diff (the dog that didn't bark). + never showed up in the diff (the dog that didn't bark). A missing + retry/redelivery spec is an absence ONLY for a handler that carries state + or side effects across attempts; a handler that is a pure function of the + request plus a conditional store predicate owes none. - **Scope drift / out-of-scope** — the diff touches what the plan's scope notes say not to touch, or quietly does extra unplanned work. - **Interface/approach substitution** — the plan named an interface, diff --git a/assets/skills/code-review/SKILL.md b/assets/skills/code-review/SKILL.md index 920d062..d82a2be 100644 --- a/assets/skills/code-review/SKILL.md +++ b/assets/skills/code-review/SKILL.md @@ -89,6 +89,15 @@ These are review findings that only surface in a diff context, not in a whole-fi finding (dead weight or a forgotten wiring step). Used in SOME applicable in-diff sites but not others = a finding naming the sites that didn't adopt it — half-adopted artifacts are how two mechanisms for the same job end up coexisting forever. +- **"Ignored / redundant request field" claims need a wire-validation check first** — before + calling a request/message field ignored, redundant, or dead on some code path because the + handler never reads it there, `fs_grep` the boundary contract for that field: proto validation + rules (`(validate.rules)`, `buf.validate`), OpenAPI/JSON-schema `required`, struct-tag + `required`/`binding`/serde attributes, or a hand-written validator/interceptor. A field the + validator MANDATES is load-bearing at the boundary even when the handler ignores it — the + honest finding (if any) is "validator requires `` on the `` path that never reads + it (`path:line`); relax with `ignore_empty`/`oneof` or document why", never "`` is + ignored". No validation found ⇒ the ignored-field finding stands, citing the grep you ran. - **Version-literal consistency and freshness** — when the diff bumps a version (image tag, tool version, dependency pin), `fs_grep` the repo for other occurrences of the OLD version string (compose files, workflows, docs, sibling Dockerfiles) — stragglers are findings. For