gitea: subscribe the review hook to pull_request_review
The hook registered with no events at all and delivered nothing. "pull_request_review_comment" and "pull_request_review_rejected" are real HookEventTypes and real X-GitHub-Event-Type values, but they are not things gitea's hook API accepts. updateHookEvents (routers/api/v1/utils/hook.go) matches a fixed list of api names and silently ignores anything else, so every event flag stayed false, the POST succeeded, and the hook sat there inert. There is no narrower api name: HasEvent (models/webhook/webhook.go) collapses approved, rejected and review-comment onto HookEventPullRequestReview, so `pull_request_review` is a single switch for all three. Approvals consequently cannot be excluded at the hook any more. They now cross the wire as "pull_request_approved", which is not in the route's event list, so Hermes ignores them on the event match -- before the filter script and before any LLM call. Gitea's delivery log will show them answered 200/ignored, which is intended. That makes three namespaces for the same event rather than two, so the tables in both nix files and the README now carry the api column, and the README warns about the silent-ignore behaviour that hid this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94o42aQ8VkBmEWvDem5xa
This commit is contained in:
Claude Opus 5
@@ -42,13 +42,16 @@ run. Each service module opens its own firewall ports.
|
||||
Jupiter's Gitea registers one webhook per Hermes route, straight at Hermes on
|
||||
mars (`http://mars.orbit.sol:8644/webhooks/<route>`), with no relay in between:
|
||||
|
||||
| route | subscribed gitea events | wakes luna on |
|
||||
| route | gitea hook event | wakes luna on |
|
||||
| --- | --- | --- |
|
||||
| `gitea-pr-comments` | `pull_request_comment` | a timeline comment on a PR |
|
||||
| `gitea-pr-reviews` | `pull_request_review_comment`, `pull_request_review_rejected` | a review with a body, or changes requested |
|
||||
| `gitea-pr-reviews` | `pull_request_review` | a review with a body, or changes requested |
|
||||
|
||||
Approvals are deliberately not subscribed: an approval is darman signing off,
|
||||
not asking for work. Gitea's `addDefaultHeaders` signs every webhook type with
|
||||
Approvals cannot be excluded at the hook — `pull_request_review` is one switch
|
||||
for all three review types — so they are delivered and then dropped by the
|
||||
Hermes route, which does not list `pull_request_approved`. Expect them in
|
||||
gitea's delivery log answered 200/ignored; that is the design, not a failure.
|
||||
Gitea's `addDefaultHeaders` signs every webhook type with
|
||||
`X-Hub-Signature-256` in GitHub's exact format and sends `X-GitHub-Event`
|
||||
unconditionally — which is exactly what Hermes validates against the route
|
||||
secret and reads the event name from, so the two speak the same protocol
|
||||
@@ -61,17 +64,24 @@ defaults to `external` and does NOT include tailnet addresses
|
||||
gitea classifies it). `services/dev/gitea.nix` sets it accordingly; without
|
||||
that, deliveries fail with `webhook can only call allowed HTTP servers`.
|
||||
|
||||
Gitea names webhook events twice, and the two namespaces collide. The hook's
|
||||
`events` array takes the *subscription* name; `X-GitHub-Event`, which is what
|
||||
each Hermes route matches its `events` against, carries a lossy *wire* name
|
||||
from `HookEventType.Event()`:
|
||||
Gitea spells the same event three ways, and two of the spellings collide. The
|
||||
hook's `events` array takes an *api* name (`updateHookEvents` in
|
||||
`routers/api/v1/utils/hook.go`), which is a coarser set than the internal
|
||||
`HookEventType`; `X-GitHub-Event`, which is what each Hermes route matches its
|
||||
`events` against, carries a lossy *wire* name from `HookEventType.Event()`:
|
||||
|
||||
| subscription | wire | what it is |
|
||||
| HookEventType | wire (mars route) | api (gitea hook) |
|
||||
| --- | --- | --- |
|
||||
| `pull_request_comment` | `issue_comment` | comment on a PR |
|
||||
| `pull_request_review_comment` | `pull_request_comment` | review with a body |
|
||||
| `pull_request_review_rejected` | `pull_request_rejected` | changes requested |
|
||||
| `pull_request_review_approved` | `pull_request_approved` | approval |
|
||||
| `issue_comment` | `issue_comment` | `issue_comment` |
|
||||
| `pull_request_comment` | `issue_comment` | `pull_request_comment` |
|
||||
| `pull_request_review_comment` | `pull_request_comment` | `pull_request_review` |
|
||||
| `pull_request_review_rejected` | `pull_request_rejected` | `pull_request_review` |
|
||||
| `pull_request_review_approved` | `pull_request_approved` | `pull_request_review` |
|
||||
|
||||
Watch the api column: `updateHookEvents` **silently ignores strings it does not
|
||||
recognise**, so a plausible-looking name that is a valid `HookEventType` but
|
||||
not a valid api event leaves the hook registered with no events at all — no
|
||||
error, no deliveries. Check a new hook's event list in the UI after adding it.
|
||||
|
||||
So `services/dev/gitea.nix` and `hosts/mars/hermes-agent.nix` deliberately name
|
||||
the same event differently, and neither is a typo. `X-GitHub-Event-Type`
|
||||
|
||||
@@ -27,9 +27,14 @@ this route subscribes to map back to a review type here:
|
||||
pull_request_comment pull_request_review_comment review with a body
|
||||
pull_request_rejected pull_request_review_rejected changes requested
|
||||
|
||||
Approvals (wire pull_request_approved) are not subscribed, so
|
||||
pull_request_review_approved is not in ALLOWED_REVIEW_TYPES: an approval is
|
||||
darman signing off, not asking for work. Add both to widen it.
|
||||
Approvals DO reach the gitea hook: its api-level `pull_request_review` event
|
||||
is a single switch for all three review types and cannot be narrowed (HasEvent
|
||||
in models/webhook/webhook.go collapses them onto it). They get dropped one
|
||||
step earlier than this script instead -- "pull_request_approved" is not in the
|
||||
route's event list, so Hermes ignores those deliveries on the event match,
|
||||
before the script runs. That is why pull_request_review_approved is absent
|
||||
from ALLOWED_REVIEW_TYPES below: an approval is darman signing off, not asking
|
||||
for work. Widening means adding it in both places.
|
||||
"""
|
||||
import json
|
||||
import sys
|
||||
|
||||
+24
-14
@@ -367,24 +367,34 @@ in
|
||||
# the file (mtime-gated) on the next delivery — no container restart, and no
|
||||
# `podman exec` quoting chain between nix and the prompt text.
|
||||
#
|
||||
# Events are WIRE names (X-GitHub-Event), not subscription names. Gitea uses
|
||||
# the same strings in two namespaces and they collide — from
|
||||
# HookEventType.Event() in modules/webhook/type.go:
|
||||
# Events are WIRE names (X-GitHub-Event). Gitea spells the same events three
|
||||
# different ways and two of the spellings collide — from
|
||||
# HookEventType.Event() in modules/webhook/type.go, and updateHookEvents in
|
||||
# routers/api/v1/utils/hook.go for the api column:
|
||||
#
|
||||
# subscription name wire name what it is
|
||||
# --------------------------- ---------------------- ------------------
|
||||
# issue_comment issue_comment comment on an issue
|
||||
# pull_request_comment issue_comment comment on a PR
|
||||
# pull_request_review_comment pull_request_comment review with a body
|
||||
# pull_request_review_rejected pull_request_rejected changes requested
|
||||
# pull_request_review_approved pull_request_approved approval
|
||||
# HookEventType wire name (here) api name (gitea.nix)
|
||||
# --------------------------- ---------------------- --------------------
|
||||
# issue_comment issue_comment issue_comment
|
||||
# pull_request_comment issue_comment pull_request_comment
|
||||
# pull_request_review_comment pull_request_comment pull_request_review
|
||||
# pull_request_review_rejected pull_request_rejected pull_request_review
|
||||
# pull_request_review_approved pull_request_approved pull_request_review
|
||||
#
|
||||
# The hooks' `events` arrays in services/dev/gitea.nix take the SUBSCRIPTION
|
||||
# name; Hermes matches these against X-GitHub-Event, i.e. the WIRE name. So
|
||||
# Hermes matches these against X-GitHub-Event, i.e. the WIRE name. So
|
||||
# "pull_request_comment" HERE means a review and "issue_comment" HERE means
|
||||
# a comment — the exact inversion of how they read. X-GitHub-Event-Type
|
||||
# carries the subscription name, but Hermes does not look at it. Both files
|
||||
# therefore name the same event differently on purpose; neither is a typo.
|
||||
# carries the HookEventType, but Hermes does not look at it. This file and
|
||||
# services/dev/gitea.nix therefore name the same event differently on
|
||||
# purpose; neither is a typo.
|
||||
#
|
||||
# The api column is not a third alias but a coarser set: HasEvent
|
||||
# (models/webhook/webhook.go) collapses all three review types onto
|
||||
# pull_request_review, so the gitea hook cannot subscribe them separately.
|
||||
# Approvals arrive here as a result and are dropped by NOT being in
|
||||
# prReviewEvents — Hermes answers {"status": "ignored"} on the event match,
|
||||
# before the filter script and before any LLM call. Widening to approvals is
|
||||
# a mars-side change only: add "pull_request_approved" to prReviewEvents and
|
||||
# "pull_request_review_approved" to the filter's ALLOWED_REVIEW_TYPES.
|
||||
#
|
||||
# issue_comment on the wire covers comments on plain issues too; the hook
|
||||
# does not subscribe those, and the comment filter's is_pull check drops
|
||||
|
||||
+28
-22
@@ -25,30 +25,36 @@ let
|
||||
# dispatches on (http://mars.orbit.sol:8644/webhooks/<route>), so it must
|
||||
# match a key in the route config that hosts/mars/hermes-agent.nix writes.
|
||||
#
|
||||
# `events` are SUBSCRIPTION names, and gitea reuses these strings in a
|
||||
# second, colliding namespace on the wire — see the long comment on the
|
||||
# route unit in hosts/mars/hermes-agent.nix. "pull_request_comment" HERE
|
||||
# means a timeline comment on a pull request; the same string in
|
||||
# X-GitHub-Event means a review. The two files therefore name the same
|
||||
# event differently on purpose, and neither is a typo:
|
||||
# `events` are the strings gitea's HOOK API accepts. That set is coarser
|
||||
# than gitea's internal HookEventType set, and both collide on spelling with
|
||||
# the wire names Hermes matches on — three namespaces, one of which is a
|
||||
# trap. From routers/api/v1/utils/hook.go (updateHookEvents),
|
||||
# models/webhook/webhook.go (HasEvent) and modules/webhook/type.go (Event()):
|
||||
#
|
||||
# here (subscription) there (route events)
|
||||
# ---------------------------- --------------------
|
||||
# pull_request_comment issue_comment
|
||||
# pull_request_review_comment pull_request_comment
|
||||
# pull_request_review_rejected pull_request_rejected
|
||||
# api event (here) delivers wire name (mars route)
|
||||
# -------------------- ------------------- ----------------------
|
||||
# pull_request_comment comment on a PR issue_comment
|
||||
# pull_request_review review with a body pull_request_comment
|
||||
# changes requested pull_request_rejected
|
||||
# approval pull_request_approved
|
||||
#
|
||||
# Hermes would drop everything else anyway (each route matches on
|
||||
# X-GitHub-Event before any LLM call, and then runs a filter script), so
|
||||
# subscribing narrowly here is defence in depth rather than the only gate:
|
||||
# it keeps traffic that can never be acted on from crossing the wire and
|
||||
# reaching the agent's process at all.
|
||||
# So this file and hosts/mars/hermes-agent.nix name the same event
|
||||
# differently on purpose, and neither is a typo.
|
||||
#
|
||||
# Approvals (pull_request_review_approved) are deliberately absent: an
|
||||
# approval is darman signing off, not asking for work, and waking an agent
|
||||
# run on every LGTM is pure cost. Adding it means adding it BOTH here and
|
||||
# to prReviewEvents/ALLOWED_REVIEW_TYPES on mars — as "pull_request_approved"
|
||||
# there, per the table above.
|
||||
# THE TRAP: updateHookEvents silently ignores strings it does not recognise,
|
||||
# so a plausible-looking but non-API name leaves the hook registered with no
|
||||
# events at all, delivering nothing and reporting no error. That is exactly
|
||||
# what "pull_request_review_comment" did here — a real HookEventType, and a
|
||||
# real value of X-GitHub-Event-Type, but not an API event name.
|
||||
#
|
||||
# There is no narrower name for reviews: HasEvent collapses approved,
|
||||
# rejected and review-comment onto HookEventPullRequestReview, so
|
||||
# `pull_request_review` is a single switch for all three. Approvals
|
||||
# therefore cannot be excluded here. They are dropped on the mars side
|
||||
# instead — the route's event list has no "pull_request_approved", so Hermes
|
||||
# answers {"status": "ignored"} without running the filter or spending a
|
||||
# token. Expect approvals in gitea's delivery log, answered 200 and ignored;
|
||||
# that is the design, not a failure.
|
||||
giteaHermesHooks = [
|
||||
{
|
||||
name = "PR comments Hermes";
|
||||
@@ -58,7 +64,7 @@ let
|
||||
{
|
||||
name = "PR reviews Hermes";
|
||||
route = "gitea-pr-reviews";
|
||||
events = [ "pull_request_review_comment" "pull_request_review_rejected" ];
|
||||
events = [ "pull_request_review" ];
|
||||
}
|
||||
];
|
||||
in
|
||||
|
||||
Reference in New Issue
Block a user