harden(spec-drift): stop the recurring #467/#470 spam + close drift-test false-negatives (#471)

* ci(spec-drift): stop weekly/daily issue spam — ack/snooze, auto-close, content titles

The recurring #467 (nightly) / #470 (weekly) issues are two detectors on one
real, deterministic upstream drift — but the reporting mechanics amplified the
pain. Fix the mechanics without weakening the read-only security model.

Nightly (spec-drift.yml):
- Comment only when the failing-test-ID set changes (a fingerprint), not every
  failing night — a deterministic upstream produced identical spam indefinitely.
- `spec-drift-ack` / `snoozed` label silences comments while a reconcile is in
  flight, without closing the tracker.
- New report-success job auto-closes the tracker on a green scheduled run
  (the workflow previously only ever opened/commented).

Weekly (spec-sync.yml):
- Title leads with a content signal ([content-changed] + path delta + short
  fingerprint) instead of a bare version — Kalshi changes content in place
  without bumping info.version, so "3.23.0 → 3.23.0" read as a false alarm.
- Supersede stale weekly snapshots so exactly one weekly drift issue stays open
  instead of one per upstream micro-revision. Scoped to spec-sync-bot so the
  nightly tracker is never touched.

Refs #467, #470

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(contracts): de-brittle drift suite + close false-negative gaps

Direction 1 — less brittle (stop benign churn redding CI against a fast-moving,
loosely-versioned upstream):
- Additive drift split: a spec-REQUIRED field the SDK lacks still hard-fails
  (act now); a spec-OPTIONAL field becomes a non-blocking
  AdditiveOptionalDriftWarning. It deliberately subclasses Warning (not
  UserWarning) so the nightly `-W error::UserWarning` gate cannot promote benign
  optional drift to a red build. +3 regression tests lock the contract.
- test_schema_coverage batches all unresolved schemas into one failure instead
  of failing on the first (one-at-a-time reruns on multi-removal drift).

Direction 2 — close false-negatives (catch real drift the suite missed):
- test_every_spec_endpoint_is_mapped asserts every spec OPERATION
  (core+perps+perps_scm) is in a METHOD_ENDPOINT_MAP, minus a reasoned
  _UNIMPLEMENTED_ENDPOINTS allowlist. Catches ADDED endpoints/operations the
  schema/path-count diff misses — verified against live 3.24.0, which had it
  caught the two new /margin/* endpoints AND a new GET on an existing
  /communications/rfqs/.../quotes path that the weekly path diff reported as
  "Added endpoints: (none)".
- kwarg_rename exclusions are now spec-anchored (test_exclusion_map_is_current):
  the spec-side param must still exist for the endpoint, so a rename can't go
  stale and silently mask an upstream param removal/rename.

Docstring updated to the actual post-#171/#172 behavior and to document why
response-side field REMOVAL is intentionally NOT gated: the vendored "Kalshi
Trade API Manual Endpoints" spec is a curated subset, so SDK response models are
a superset by design; gating removals would red CI on every intentional SDK
field. Request-BODY drift already gates removals (_check_model_drift), which is
where a rename's dropped old name is caught.

Refs #467, #470

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: TexasCoding

.github/workflows/spec-drift.yml

@@ -85,6 +85,14 @@ jobs:
             > /tmp/drift.log 2>&1 || true
           # Keep the last 200 lines so the body stays inside GitHub's limits.
           tail -n 200 /tmp/drift.log > /tmp/drift.tail
+          # Failing-set fingerprint: sha256 over the sorted, unique failing test
+          # IDs. Deterministic upstream yields an identical set every night, so
+          # the tracker step below re-comments only when this fingerprint CHANGES
+          # (kills the daily "Still failing" spam that made #467 noisy).
+          grep -oE '^FAILED [^ ]+' /tmp/drift.log | awk '{print $2}' | sort -u \
+            > /tmp/fail_ids.txt || true
+          fail_fp=$(sha256sum /tmp/fail_ids.txt | awk '{print $1}')
+          echo "fail_fp=${fail_fp}" >> "$GITHUB_OUTPUT"
 
       - name: Ensure spec-drift label exists
         env:
@@ -100,11 +108,12 @@ jobs:
         env:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
+          FAIL_FP: ${{ steps.repro.outputs.fail_fp }}
         run: |
           set -euo pipefail
 
           # Stable marker (NOT a content fingerprint): exactly one open tracker
-          # at a time. Re-runs append a dated comment instead of duplicating.
+          # at a time. Re-runs comment only when the FAILING SET changes.
           readonly MARKER='nightly-spec-drift-tracker'
           today=$(date -u '+%Y-%m-%d')
 
@@ -120,34 +129,99 @@ jobs:
           # heredoc) so upstream-sourced log text is never shell-expanded.
           {
             printf '## Nightly strict contract tests failed\n\n'
-            printf 'The scheduled `Spec Drift Detection` run promoted additive drift to an error.\n'
-            printf 'This means upstream OpenAPI/AsyncAPI changed and the SDK contract suite no longer matches.\n\n'
-            printf '**To resolve:** locally run `uv run python scripts/sync_spec.py` + `uv run python scripts/generate.py`, reconcile models/maps, and open a PR with `Closes #<this issue>`.\n\n'
+            printf 'The scheduled `Spec Drift Detection` run found the SDK contract suite no longer matches upstream OpenAPI/AsyncAPI.\n\n'
+            printf '**To resolve:** locally run `uv run python scripts/sync_spec.py` + `uv run python scripts/generate.py`, reconcile models/maps, and open a PR with `Closes #<this issue>`. This tracker auto-closes on the next green scheduled run.\n\n'
+            printf '**To silence** while a reconcile is in flight, add the `spec-drift-ack` label — nightly runs then stop commenting until the failing set changes.\n\n'
             printf -- '- Failing run: %s\n\n' "${RUN_URL}"
             printf '### Failing test output (last 200 lines)\n\n```\n'
             cat /tmp/drift.tail
             printf '\n```\n'
           } > /tmp/body.md
 
           if [ -n "${existing}" ]; then
+            # Ack/snooze gate: a maintainer already on this drift can add the
+            # `spec-drift-ack` (or `snoozed`) label to silence nightly noise
+            # without closing the tracker.
+            labels=$(gh issue view "${existing}" --repo "${GITHUB_REPOSITORY}" \
+              --json labels --jq '[.labels[].name] | join(",")')
+            case ",${labels}," in
+              *,spec-drift-ack,*|*,snoozed,*)
+                echo "Tracker #${existing} is ack'd/snoozed; not commenting."
+                exit 0 ;;
+            esac
+
+            # Only comment when the FAILING SET changed. A deterministic upstream
+            # produces an identical set every night; re-posting it is spam.
+            last_fp=$(gh issue view "${existing}" --repo "${GITHUB_REPOSITORY}" \
+              --json body,comments --jq '[.body, (.comments[].body)] | .[]' \
+              | grep -oE 'fail-fingerprint:[0-9a-f]+' | tail -1 | cut -d: -f2 || true)
+            if [ "${last_fp}" = "${FAIL_FP}" ]; then
+              echo "Failing set unchanged (fingerprint ${FAIL_FP}); no new comment."
+              exit 0
+            fi
+
             {
-              printf 'Still failing as of **%s** ([run](%s)).\n\n' "${today}" "${RUN_URL}"
+              printf 'Failing set changed as of **%s** ([run](%s)).\n\n' "${today}" "${RUN_URL}"
               printf '### Latest failing output (last 200 lines)\n\n```\n'
               cat /tmp/drift.tail
               printf '\n```\n'
+              printf '\n<!-- fail-fingerprint:%s -->\n' "${FAIL_FP}"
             } > /tmp/comment.md
-            echo "Tracker already open as #${existing}; appending dated comment."
+            echo "Failing set changed (${last_fp:-none} -> ${FAIL_FP}); appending comment."
             gh issue comment "${existing}" \
               --repo "${GITHUB_REPOSITORY}" \
               --body-file /tmp/comment.md
             exit 0
           fi
 
-          # Hidden marker so the dedup query above matches future runs.
+          # Hidden markers so future runs match this tracker (MARKER) and can
+          # diff the failing set (fail-fingerprint).
           printf '\n<!-- spec-drift-bot: do not edit\n%s\n-->\n' "${MARKER}" >> /tmp/body.md
+          printf '<!-- fail-fingerprint:%s -->\n' "${FAIL_FP}" >> /tmp/body.md
 
           gh issue create \
             --repo "${GITHUB_REPOSITORY}" \
             --label spec-drift \
             --title "Nightly spec-drift: strict contract tests failing (since ${today})" \
             --body-file /tmp/body.md
+
+  # Companion to report-failure: when a scheduled strict run goes GREEN, the
+  # drift has been reconciled — close the open nightly tracker so it doesn't
+  # linger open forever (the old workflow only ever opened/commented). Least
+  # privilege: issues:write lives only on this job, and it runs no third-party
+  # actions and no untrusted upstream code (pure `gh` against the repo), so it
+  # needs neither a checkout nor SHA-pinning.
+  report-success:
+    needs: drift-check
+    if: success() && github.event_name == 'schedule'
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      issues: write
+    steps:
+      - name: Close resolved nightly drift tracker
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
+        run: |
+          set -euo pipefail
+          readonly MARKER='nightly-spec-drift-tracker'
+          today=$(date -u '+%Y-%m-%d')
+
+          existing=$(gh issue list \
+            --repo "${GITHUB_REPOSITORY}" \
+            --state open \
+            --label spec-drift \
+            --limit 200 \
+            --json number,body \
+            --jq '[.[] | select(.body | contains("'"${MARKER}"'"))] | first | .number // empty')
+
+          if [ -z "${existing}" ]; then
+            echo "No open nightly tracker; nothing to close."
+            exit 0
+          fi
+
+          gh issue close "${existing}" \
+            --repo "${GITHUB_REPOSITORY}" \
+            --reason completed \
+            --comment "Resolved — strict contract suite green as of ${today} ([run](${RUN_URL})). Auto-closed by the nightly spec-drift workflow."

.github/workflows/spec-sync.yml

@@ -225,14 +225,16 @@ jobs:
             "${PERPS_ASYNCAPI_SHA}" "${PERPS_SCM_SHA}" | sha256sum | awk '{print $1}')
           export FINGERPRINT
 
-          # Dedup: skip if an open spec-drift issue already embeds this fingerprint.
+          # Dedup: skip if an open weekly (spec-sync-bot) drift issue already
+          # embeds this fingerprint. Scoped to spec-sync-bot so the nightly
+          # tracker (spec-drift-bot) is never mistaken for a weekly snapshot.
           existing=$(gh issue list \
             --repo "${GITHUB_REPOSITORY}" \
             --state open \
             --label spec-drift \
             --limit 200 \
             --json number,body \
-            --jq '[.[] | select(.body | contains("fingerprint:'"${FINGERPRINT}"'"))] | first | .number // empty')
+            --jq '[.[] | select(.body | contains("spec-sync-bot")) | select(.body | contains("fingerprint:'"${FINGERPRINT}"'"))] | first | .number // empty')
 
           if [ -n "${existing}" ]; then
             echo "Drift fingerprint ${FINGERPRINT} already tracked in issue #${existing}; nothing to do."
@@ -246,10 +248,40 @@ jobs:
           python3 scripts/render_drift_body.py > "${body_file}"
 
           today=$(date -u '+%Y-%m-%d')
-          title="Spec drift ${today}: openapi ${OLD_VERSION} → ${NEW_VERSION}"
+          short_fp=${FINGERPRINT:0:8}
+          # Lead the title with a CONTENT signal, not just info.version: Kalshi
+          # changes spec content in place without bumping the version, so a bare
+          # "3.23.0 → 3.23.0" reads like a no-op false alarm. Tag content-only
+          # changes and always surface the path delta + short fingerprint.
+          if [ "${OLD_VERSION}" = "${NEW_VERSION}" ]; then
+            title="Spec drift ${today}: openapi ${NEW_VERSION} [content-changed] (paths ${OLD_PATH_COUNT}→${NEW_PATH_COUNT}, fp ${short_fp})"
+          else
+            title="Spec drift ${today}: openapi ${OLD_VERSION} → ${NEW_VERSION} (paths ${OLD_PATH_COUNT}→${NEW_PATH_COUNT}, fp ${short_fp})"
+          fi
 
-          gh issue create \
+          new_url=$(gh issue create \
             --repo "${GITHUB_REPOSITORY}" \
             --label spec-drift \
             --title "${title}" \
-            --body-file "${body_file}"
+            --body-file "${body_file}")
+          echo "Opened ${new_url}"
+
+          # Supersede older weekly drift snapshots whose fingerprint no longer
+          # matches live upstream — keep exactly one open weekly issue instead of
+          # piling up one per upstream micro-revision. Scoped to spec-sync-bot so
+          # the nightly tracker is never touched here; the freshly-created issue
+          # carries the current fingerprint and is therefore excluded.
+          stale=$(gh issue list \
+            --repo "${GITHUB_REPOSITORY}" \
+            --state open \
+            --label spec-drift \
+            --limit 200 \
+            --json number,body \
+            --jq '[.[] | select(.body | contains("spec-sync-bot")) | select((.body | contains("fingerprint:'"${FINGERPRINT}"'")) | not) | .number] | .[]')
+          for n in ${stale}; do
+            gh issue close "${n}" \
+              --repo "${GITHUB_REPOSITORY}" \
+              --reason "not planned" \
+              --comment "Superseded by ${new_url} — upstream specs moved again, so this drift snapshot is stale. Reconcile against current upstream."
+            echo "Superseded #${n}"
+          done