Skip to content

refactor(webhook): extract webhook API router + profile admission (Webhook Feature Package) - #84835

Closed
andrexibiza wants to merge 9 commits into
NousResearch:mainfrom
andrexibiza:campaign/webhook-revolution-b3aa561f
Closed

andrexibiza wants to merge 9 commits into
NousResearch:mainfrom
andrexibiza:campaign/webhook-revolution-b3aa561f

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Part of the Webhook Feature Package Feature Package.

Behavior-preserving extractions from the webhook god-files, rebased onto current main (1d3d021):

  1. refactor(web): extract webhook API router without behavior change — moves the webhook REST cluster from hermes_cli/web_server.py (18K-line god-file) into hermes_cli/web_routers/webhooks.py, re-exporting patched names through hermes_cli.web_server for existing tests/users.
  2. fix(web): preserve webhook summary monkeypatch seam — keeps the summary endpoint monkeypatch-compatible.
  3. refactor(webhook): extract profile admission policy — moves _resolve_request_profile / _route_allows_profile / _PROFILE_REJECTED into gateway/platforms/webhook_profile_admission.py as WebhookProfileAdmissionMixin, composed into WebhookAdapter MRO.
  4. fix(webhook): preserve profile annotation resolution — preserves profile annotation behavior.

Verification

  • tests/gateway/test_webhook_profile_admission_seam.py + tests/test_web_server_webhooks_seam.py5 passed on rebased base.
  • No behavior change: verbatim extraction, seam identity preserved, monkeypatch surfaces tested.

2K invariant

web_server.py reduced 161 lines; webhook.py reduced 70 lines. No file exceeds 2,000 lines.

Fixes #85054

@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery comp/cli CLI entry point, hermes_cli/, setup wizard 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 sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades needs-decision Awaiting maintainer decision before any implementation labels Aug 12, 2026
@andrexibiza

Copy link
Copy Markdown
Contributor Author

Class closure — selective multiplex admission authority

Production chain

WebhookAdapter._resolve_request_profile() reads the authoritative gateway config and calls profiles_to_serve(multiplex=True, profile_allowlist=cfg.multiplex_profile_allowlist). The extraction had dropped that second argument. Because None means “serve every named profile,” the omission silently broadened a configured selective allowlist.

The repair belongs at that extracted authority boundary—not in downstream route checks and not as an independently reconstructed policy—so every consumer observes the same configuration authority.

Evidence chain

  • Exact defective parent: ececc7c02563e53c942a7dbe9acdc3b5b49da807.
  • RED reproduction: observed [(True, None)]; expected [(True, ["worker"])].
  • Canonical final head/tree: 898a3f008259a8620d8d5f6eb2e1bf9e5f2b3179 / 1a07947ec6abcac8b573ced206a37695ec3b6c28.
  • Final exact-head matrix: 49/49 focused tests passed—seven tests on each of seven repaired PR heads.
  • Canonical broader webhook selection: 43/43 passed.
  • Ruff check/format, compilation, git diff --check, attribution, and current-main composition passed.
  • Semantic replay patch ID: b1586ab2accb0fc7516984b32a694f8fcee37237.
  • Format-only patch ID: 99c7a488ddb880e1b249ae8cf0be02d94fb83352; AST-identical on all seven branches.
  • Two independent final-object reviews: APPROVED at 898a3f00… / 1a07947e….
  • Current-head CI: SUCCESS on all seven PRs.
  • Published graph readback: issue Webhook profile admission drops selective multiplex allowlist across extracted branches #85054 sees EPIC Webhook Feature Package — graph-gated repair (meta-issue) #84834 and all seven PRs; receipt SHA-256 bb1b819db783d98cbcc4665caac020549a7c5f4f865c8244680a20543fc13139.

Sibling map

PR Surface Final head Class edge CI
#84835 Profile-admission origin 898a3f008259 Fixes #85054
#84849 Auth extraction descendant 5f270716c07b Related #85054
#84858 Rendering extraction descendant fad39719caa0 Related #85054
#84939 Delivery extraction descendant 4594059f0c7f Related #85054
#84978 Route/store descendant 41152d37e553 Related #85054
#85002 Effective-config descendant 51912f436ed1 Related #85054
#85025 Secret-migration descendant 20a837ad1c77 Related #85054

Issue #85054 remains open behind this PR’s native Fixes edge so maintainer merge closes it through GitHub’s canonical path. The contributor-side defect class is complete across every live carrier.

Signed-off-by: Axl Ibiza, MBA <andrexibiza@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 85de2235-584e-4c4d-9a16-61731c4ff411

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@andrexibiza andrexibiza changed the title refactor(webhook): extract webhook API router + profile admission (Webhook Revolution) refactor(webhook): extract webhook API router + profile admission (Webhook Feature Package) Aug 15, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

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

refactor(webhook): extract webhook API router + profile admission (Webhook Feature Package)

Solid decomposition of the webhook god-file; the late() binding seam keeps monkeypatch-transparency. Points:

  1. Machine-specific artifacts committedartifacts/webhook-repair/30e9449403c6448d24036ecf31b7f9ceaf67b769/task-04/*.json contain absolute Windows paths (D:\HERMES-TEMP\webhook-revolution\task-04-rest-router) plus commit hashes and timestamps. These are working artifacts of the automation that produced the PR and should not be part of the repo (or should live under a git-ignored path). Please drop them from the PR.

  2. Router extraction is duplicated across PRs in this seriesrefactor(webhook): extract webhook API router + profile admission (Webhook Feature Package) #84835, refactor(webhook): extract signature validation into webhook_auth mixin (Webhook Feature Package) #84849, refactor(webhook): extract prompt rendering into webhook_rendering mixin (Webhook Feature Package) #84858 and refactor(webhook): extract delivery into webhook_delivery mixin (Webhook Feature Package) #84939 each add the byte-identical hermes_cli/web_routers/webhooks.py + web_server.py change (same files, same content, same base commit). Merging any one makes the others conflict. Coordinate so the router extraction lands exactly once (e.g., in the first PR of the series) and the rest rebase onto it.

  3. create_webhook — None-guard gap (pre-existing, now touchable): hermes_cli/web_routers/webhooks.py: "events": [e.strip() for e in body.events if e.strip()] raises TypeError when body.events is None (same for body.skills). Since the router is being extracted anyway, a (body.events or []) guard would harden the dashboard endpoint against partial payloads.

  4. MRO test brittlenesstests/gateway/test_webhook_profile_admission_seam.py asserts an exact __mro__[:3] prefix; later PRs in this series add more mixins and had to relax this to a membership assertion. Consider the membership form now to avoid churn.

@alt-glitch alt-glitch removed the needs-decision Awaiting maintainer decision before any implementation label Aug 15, 2026
@andrexibiza

Copy link
Copy Markdown
Contributor Author

The webhook API router extraction is complete at exact head 159d70a78f052c40d8c4d8f2ff0a94a276fc4b9f. 159d70a78f05 records the behavior-preserving move of the REST cluster into hermes_cli/web_routers/webhooks.py, with web_server.py re-exporting the patched names used by existing callers and tests. The resulting surface keeps the profile-admission and summary-resolution seams intact, so endpoint routing can be separated from the large server module without changing request behavior. GitHub check rollup on this exact head: SUCCESS.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @andrexibiza for this refactor. Closing as superseded: the webhook REST cluster left hermes_cli/web_server.py on main via direct commits (0cca195) as hermes_cli/web_routers/ops.py (list_webhooks/enable_webhooks/create_webhook/delete_webhook), and the profile-admission logic (_resolve_request_profile, _route_allows_profile, _handle_profile_ingress with multiplex_profiles gating — 8c5e846) is implemented directly on WebhookAdapter in gateway/platforms/webhook.py; #85054 is closed as completed. Appreciate the work.

@teknium1 teknium1 closed this Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/webhook Webhook / API server sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Webhook profile admission drops selective multiplex allowlist across extracted branches

4 participants