Skip to content

fix(webhook): close Task 10 intake identity and raw-bound contracts - #90236

Open
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:task10-current-main-closure
Open

fix(webhook): close Task 10 intake identity and raw-bound contracts#90236
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:task10-current-main-closure

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Canonical closure candidate for Webhook Feature Package Task 10. This branch is semantically compressed directly onto exact upstream main@d5281f59819d2ea2ce6754faec2ce317c92366c8 as one commit. It does not replay historical #85523 topology or carry campaign receipts, attribution churn, or patch-applier machinery.

Exact topology

  • base/parent: d5281f59819d2ea2ce6754faec2ce317c92366c8
  • head: b0f8dab4029e53eab4b8c88ea9b9a3d4638b3eb1
  • tree: 1564dab72a19e4efaa0bb1001bf6e191a2a7bb27
  • one commit, three files, +640/-81

Owned files:

  • gateway/platforms/webhook.py
  • tests/gateway/test_webhook_adapter.py
  • tests/gateway/test_webhook_task10_current_main.py

The composition preserves current-main #91583 profile authority: with multiplexing disabled, only a prefix matching the serving Hermes home may fall through; every other /p/<profile>/ prefix fails closed through _PROFILE_REJECTED.

Closed contracts

Provider-native intake identity

  • resolve stable retry identities for GitHub, Svix, GitLab, Standard Webhooks, generic request IDs, explicitly declared Stripe event IDs, and explicitly declared Chatwoot identities;
  • never synthesize a timestamp as an idempotency identity;
  • when no stable provider identity exists, generate a unique trace/session UUID and deliberately skip retry deduplication;
  • scope keys by (profile, route, provider, delivery_id);
  • bind each stable identity to the original body hash;
  • return explicit accepted, duplicate, or conflict truth, including HTTP 409 for same-key/different-body reuse;
  • serialize same-key mutation through a re-entrant lock.

Bounded state and rate authority

  • normalize idempotency_max_entries deterministically;
  • enforce the configured hard ceiling at insertion time, including values below the old implicit 128-entry floor;
  • prune body-hash state together with identity state;
  • isolate rate windows by (profile, route) and return Retry-After on 429.

Strict HTTP intake

  • reject non-identity Content-Encoding;
  • parse only the declared media type;
  • return 415 for missing or unsupported media types;
  • never fall through from invalid JSON into form parsing;
  • require object-shaped webhook envelopes.

UTF-8 complete-envelope raw bounds

  • {__raw__} emits a complete JSON envelope capped at 4,000 UTF-8 bytes;
  • {__raw__:N} supports explicit 64–1,000,000-byte caps;
  • metadata and JSON escaping count toward the selected cap;
  • truncation never splits UTF-8 and always remains parseable JSON;
  • raw payload substitution occurs exactly once, preventing brace-shaped payload data from becoming a second template layer.

Exact-head verification

At exact head b0f8dab4029e53eab4b8c88ea9b9a3d4638b3eb1:

tests/gateway/test_webhook_task10_current_main.py pins profile/route/provider/id isolation, body-conflict truth, same-key concurrency, insertion-time cache ceilings, malformed ceiling normalization, byte-capped parseable envelopes, invalid-cap fail-closed behavior, and one-pass substitution.

No status is inherited from the previous head, #85523, or an adjacent PR. Upstream main remained the exact parent through post-run read-back.

Interlocks / provenance

Maintainer handoff

One commit, three owned files, zero unresolved review threads, exact-head green, exact-current-main parentage, and GitHub mergeable. Task 10 has no remaining code, execution, or topology blocker.

@andrexibiza
andrexibiza force-pushed the task10-current-main-closure branch from 7f57a99 to 1cfb810 Compare August 19, 2026 19:53
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/webhook Webhook / API server sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 19, 2026
@andrexibiza
andrexibiza marked this pull request as ready for review August 19, 2026 22:04
@andrexibiza
andrexibiza force-pushed the task10-current-main-closure branch 2 times, most recently from d6968c5 to 09fef5b Compare August 20, 2026 14:35

Copy link
Copy Markdown
Contributor Author

Architecture/topology interlock: #90049 now treats this PR as the canonical Task 10 webhook node (superseding historical #85523), with terminal delivery/result truth owned separately by #85644 or its current-main successor.

Task 10 should therefore preserve exact operation/route/provider identity needed by downstream result accounting, but should not claim end-to-end success semantics that belong to Task 13. Intake acceptance is not delivery completion; the campaign can only claim terminal result truth once the fan-out layer preserves per-target terminal outcomes without collapsing partial/unknown state into success.

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Canonical Task 10 receipt: this one-commit current-main PR supersedes #85523 as the live intake/HTTP-contract owner in the webhook train. Composition order is #85002 (or compressed successor preserving effective-config ownership) → #90236#85318#90304#85644#85638#85640. #85523 remains historical provenance only; no downstream lane should reintroduce its branch topology. Task 19 alone performs final lane assembly.

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head closure review at 09fef5bbc60fe37ca47192dbb9afabe4a6fa6d2c: this is the canonical Task 10 intake owner and the semantic compression is sound. The current three-file diff preserves provider-native delivery identity, composite (profile, route, provider, delivery_id) idempotency plus body-conflict detection, profile/route rate isolation, strict JSON-object intake, and UTF-8 byte-bounded structurally valid prompt envelopes. Exact-head CI 32381041406, Docker 32381040709, and Nix 32381040529 are green. No remaining code-review blocker on Task 10 itself. Downstream #85318 must compose onto this exact contract and must not remove hashlib, because Task 10 now owns the request-body digest used by the idempotency ledger.

andrexibiza added a commit to andrexibiza/hermes-agent that referenced this pull request Aug 20, 2026
Centralize route/provider/verifier binding, provider-native retry identity, provider-scoped event extraction, and immutable intake-envelope construction behind one domain authority. Keep legacy header inference isolated to undeclared compatibility routes and never use timestamps as delivery identity.

Refs NousResearch#90989
Interlocks NousResearch#90236 and NousResearch#85318.

Copy link
Copy Markdown
Contributor Author

Architecture interlock — Task Completion Verification

This PR is the Task 10 canonical successor in Amendment I to All Gods Must Die: Adversarially Verified Transformation.

The amendment records the explicit lineage #85523 --superseded_by--> #90236 and requires all technical receipts to bind to this PR's exact head rather than the historical implementation object.

Related #80551

Copy link
Copy Markdown
Contributor Author

Task Completion Verification case-study edge

This PR is the current Task 10 canonical successor in #91230. Completion and CI receipts must bind to this PR's exact head rather than historical #85523 or any adjacent object.

Related #91230

Copy link
Copy Markdown
Contributor Author

@teknium1 maintainer review requested on exact head 416c5e48aefcccfe945f7d4118415a3b992e017a.

Revalidated now: GitHub reports open/non-draft/mergeable, with exact-head CI 32418257092, Docker 32418256106, and Nix 32418256098 all successful.

This remains the canonical executable Task 10 owner for authenticated intake/idempotency/raw-body bounds. Final webhook convergence should preserve its exact body-hash/idempotency semantics while composing #85318 verifier authority and #90995's immutable provider/intake domain contract; do not reimplement those questions in #85640.

Copy link
Copy Markdown
Contributor Author

HTTP-mechanics boundary published in #91913

The authority-continuity manifest retains this PR as the strict HTTP/raw-body/rate/idempotency owner after #90995 binds the canonical provider namespace.

Composition rule:

  • consume route/provider/verifier/delivery identity from refactor(webhook): establish canonical intake authority #90995;
  • retain media-type parsing, byte bounds, body hashing, rate windows, idempotency locking, duplicate/conflict truth, and HTTP responses here;
  • do not reintroduce a parallel provider/delivery derivation path.

Contract PR: #91913
Exact contract head: 170a3a0e67034abd7d6a2c69a16c292b4781720f.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

This closes real intake-contract holes with the right primitives: scope-keyed idempotency (profile/route/provider + delivery id) ending cross-route ID aliasing, body-hash CONFLICT as an explicit 409 instead of a silent wrong-dedupe, a true hard ceiling enforced with reserve-at-insertion, and the binary-search raw envelope that always emits parseable JSON within the byte cap. The one-winner concurrency test and envelope-cap fuzz-ish parametrization are exactly the tests this logic needs.

Two behavioral risks to weigh:

  1. Content-Type strictness breaks lenient senders (_handle_webhook, parse block): previously any body was JSON-parsed with form fallback regardless of headers; now a missing or non-JSON/non-form Content-Type returns 415. curl defaults to no Content-Type, and several SaaS senders use text/plain for JSON bodies — those integrations worked yesterday and hard-fail today, and providers often auto-disable webhooks after repeated non-2xx. Consider treating absent Content-Type as JSON (keep 415 for explicitly wrong declared types), or at minimum documenting the migration prominently.
  2. Recorded-then-failed deliveries swallow retries: _record_delivery_id now binds the key before script execution and prompt rendering. A synchronous failure after ACCEPTED — invalid template → 500, script timeout — leaves the delivery ID bound, so the provider's legitimate retry within TTL gets duplicate 200 and the event is permanently lost. The old flow checked dedup after render, so failures were retryable. Consider popping the binding on the failure paths between ACCEPTED and the async handoff (template ValueError, script non-zero), or moving ACCEPTED to just before dispatch.

Smaller notes: the header-inference precedence in _delivery_provider (gitlab before chatwoot, svix first overall) is load-bearing but implicit — a comment listing the order would prevent future reordering regressions; and validating {__raw__:N} caps at route-registration time would turn the per-request 500 into a startup-time config error. Nit: the file lost its trailing newline.

Recompose the canonical authenticated-intake slice directly on live upstream main d5281f5. Preserve the current-main non-multiplex profile-isolation boundary while closing provider-scoped idempotency, profile/route rate isolation, object-only JSON admission, and UTF-8 byte-bounded raw payload rendering.
@andrexibiza
andrexibiza force-pushed the task10-current-main-closure branch from 4dd3ff8 to b0f8dab Compare August 23, 2026 13:04

Copy link
Copy Markdown
Contributor Author

Exact-current-main completion receipt — Task 10

The canonical branch is now rewritten directly on current upstream main@d5281f59819d2ea2ce6754faec2ce317c92366c8.

This receipt supersedes every earlier head/base/run receipt on the thread and keeps #85523 as historical provenance only. No status is borrowed from an adjacent SHA. No self-approval or upstream merge was performed; independent acceptance remains with the maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/webhook Webhook / API server sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Webhook idempotency is keyed only by delivery ID, so the second route is skipped in valid multi-route fan-out

3 participants