diff --git a/README.md b/README.md index 7a5964a..2429ef4 100644 --- a/README.md +++ b/README.md @@ -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/`), 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` diff --git a/hosts/mars/gitea-pr-review-filter.py b/hosts/mars/gitea-pr-review-filter.py index 33bb27a..862a254 100644 --- a/hosts/mars/gitea-pr-review-filter.py +++ b/hosts/mars/gitea-pr-review-filter.py @@ -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 diff --git a/hosts/mars/hermes-agent.nix b/hosts/mars/hermes-agent.nix index a4d3185..4e7cd56 100644 --- a/hosts/mars/hermes-agent.nix +++ b/hosts/mars/hermes-agent.nix @@ -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 diff --git a/services/dev/gitea.nix b/services/dev/gitea.nix index cfc3feb..67f7ff1 100644 --- a/services/dev/gitea.nix +++ b/services/dev/gitea.nix @@ -25,30 +25,36 @@ let # dispatches on (http://mars.orbit.sol:8644/webhooks/), 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