From 9f40bd13f939020712c5a322fa8619e27806f322 Mon Sep 17 00:00:00 2001 From: damocles Date: Wed, 24 Jun 2026 23:07:48 +0200 Subject: [PATCH] fix(#1896): remove dead forge_notify reason drop-list + auto-unsubscribe --- docs/forge.md | 53 +++++--------------- hive-ag3nt/src/forge_notify.rs | 91 ++-------------------------------- nix/templates/harness-base.nix | 45 ----------------- nix/templates/manager.nix | 13 +---- 4 files changed, 20 insertions(+), 182 deletions(-) diff --git a/docs/forge.md b/docs/forge.md index d49e9f17..9db84275 100644 --- a/docs/forge.md +++ b/docs/forge.md @@ -104,19 +104,17 @@ the cursor to the threads still in the unread set. A failed wake delivery is left unread **and** out of the cursor, so it resurfaces next tick. -Two paths still mark-read directly (no read-before-comment value): -self-echo notifications (the agent's own writes, see below) and -`HIVE_FORGE_NOTIFY_SKIP_REASONS` drop-listed reasons. +Self-echo notifications (the agent's own writes, see below) are the +one path still marked-read directly (no read-before-comment value). > Note: the unread list grows for threads the agent never reads via > the CLI, since nothing else trims it. This does not affect guard > correctness (the guard does a per-thread, repo-scoped query) nor > wake delivery (Forgejo orders unread newest-first, so new activity -> always lands in the polled window). Bounding the unread list via a -> reason-independent firehose-reduction is a separate follow-up — the -> existing auto-unsubscribe below is gated on a `reason` field that -> this Forgejo's notification API does not actually emit, so it never -> fires today. +> always lands in the polled window). Bounding the unread list is a +> separate follow-up — explicit subscription management via a +> hive-forge CLI verb, rather than the poller second-guessing which +> repo watches to drop. ### Activation gates (graceful no-ops) @@ -240,36 +238,11 @@ fallback checks the subject payload directly. Detection is gated on `is_new` so the label only fires once on PR creation, not on every subsequent comment. -### Reason drop-list +### Subscription management -`HIVE_FORGE_NOTIFY_SKIP_REASONS` (comma-separated) suppresses -notifications whose Forgejo `reason` matches an entry. Marked-read -silently, no delivery. Drop-list is intentionally chosen over an -allow-list: - -- Allow-list would silently miss any directed signal Forgejo adds - later (`review_requested`, `mention`, future kinds). -- Drop-list explicitly identifies the noisy paths - (`subscribed`, `participating`) and lets unknown / null reasons - pass through. - -Configured per-agent via `hyperhive.forge.skipNotifyReasons` in -`agent.nix`. Default is empty (deliver everything). - -### Auto-unsubscribe on broad watches - -Default behavior: after delivering a `reason == "subscribed"` -notification, `DELETE /api/v1/repos///subscription` is -called to drop the agent's broad-watch on that repo. The agent -remains subscribed to specific issues/PRs it interacts with, but -stops receiving the firehose of every commit / new issue. - -`HIVE_FORGE_KEEP_SUBSCRIPTIONS=1` disables this — triage agents and -other firehose consumers need to keep watching every repo activity. -Set via `hyperhive.forge.keepSubscriptions = true` in `agent.nix`. - -The auto-unsub set is process-local (a `HashSet` keyed by -`owner/repo`), so a single repo only gets one DELETE per harness -boot. After harness restart the agent might re-watch the repo via -some other path; the next `subscribed` notification re-triggers the -unsubscribe. +The poller does **not** auto-unsubscribe from repo watches — it +delivers every unread notification it's handed. Bounding the +firehose (dropping broad repo watches an agent doesn't need) is done +explicitly via a hive-forge CLI subscription verb, not by the poller +guessing which watches to drop. See the `subscription` verb in +[`docs/tools/forge.md`](tools/forge.md). diff --git a/hive-ag3nt/src/forge_notify.rs b/hive-ag3nt/src/forge_notify.rs index 35db12f6..fa039ca0 100644 --- a/hive-ag3nt/src/forge_notify.rs +++ b/hive-ag3nt/src/forge_notify.rs @@ -11,9 +11,8 @@ //! //! Activation gates, self-notification filtering, body excerpt + //! truncation + heading escape, wrapper formats (comment / review / -//! new-item / state-change), meta suffix, review-request override, -//! reason drop-list, and auto-unsubscribe on broad watches all live -//! in [`docs/forge.md::Notification poller`](../../../docs/forge.md). +//! new-item / state-change), meta suffix, and review-request override +//! all live in [`docs/forge.md::Notification poller`](../../../docs/forge.md). use std::collections::{HashMap, HashSet}; use std::fmt::Write as _; @@ -119,31 +118,7 @@ pub async fn run(socket: PathBuf) { // socket becoming available right at boot. interval.tick().await; - // HIVE_FORGE_KEEP_SUBSCRIPTIONS=1 disables auto-unsubscribe for agents - // that intentionally consume the full repo notification firehose (e.g. triage). - let keep_subscriptions = std::env::var("HIVE_FORGE_KEEP_SUBSCRIPTIONS") - .is_ok_and(|v| v == "1" || v.eq_ignore_ascii_case("true")); - - // Optional reason drop-list — comma-separated Forgejo `reason` - // values to silently mark-read instead of deliver. See - // `docs/forge.md::Reason drop-list` for the drop-vs-allow rationale. - let skip_reasons: Vec = std::env::var("HIVE_FORGE_NOTIFY_SKIP_REASONS") - .unwrap_or_default() - .split(',') - .map(str::trim) - .filter(|s| !s.is_empty()) - .map(str::to_owned) - .collect(); - - if skip_reasons.is_empty() { - info!(forge_url = %forge_url, "forge_notify: polling started (all reasons)"); - } else { - info!(forge_url = %forge_url, skip = ?skip_reasons, "forge_notify: polling started"); - } - - // Repos we have already unsubscribed this process lifetime. Persists - // across polls so we don't hammer DELETE on every cycle. - let mut unsubbed_repos: HashSet = HashSet::new(); + info!(forge_url = %forge_url, "forge_notify: polling started"); // Delivery-dedupe cursor: notification thread id -> the `updated_at` // of the version we last woke the agent for. We no longer mark a @@ -165,11 +140,8 @@ pub async fn run(socket: PathBuf) { &forge_url, &token, &socket, - keep_subscriptions, - &mut unsubbed_repos, &mut delivered, &own_login, - &skip_reasons, ) .await; } @@ -739,12 +711,6 @@ fn days_from_civil(y: i64, m: i64, d: i64) -> i64 { era * 146_097 + doe - 719_468 } -#[allow( - clippy::too_many_arguments, - reason = "the notification poll's config + mutable subscription / \ - delivery-dedupe state, wired once from the poll loop; a struct \ - would just move the same fields one level out" -)] #[allow( clippy::too_many_lines, reason = "single-pass notification poll loop — split would obscure the \ @@ -756,11 +722,8 @@ async fn poll_once( forge_url: &str, token: &str, socket: &Path, - keep_subscriptions: bool, - unsubbed_repos: &mut HashSet, delivered: &mut HashMap, own_login: &str, - skip_reasons: &[String], ) { let url = format!("{forge_url}/api/v1/notifications?all=false&limit=50"); let resp = match client @@ -814,18 +777,6 @@ async fn poll_once( continue; } - // Reason drop-list: suppress noisy reasons; null/unknown pass - // through so directed signals stay deliverable (see - // `docs/forge.md::Reason drop-list`). - if !skip_reasons.is_empty() { - let reason = notif["reason"].as_str().unwrap_or(""); - if !reason.is_empty() && skip_reasons.iter().any(|r| r == reason) { - debug!(%id, %reason, "forge_notify: skipping (reason in drop-list)"); - mark_read(client, forge_url, token, id).await; - continue; - } - } - let body_opt = format_notification(client, token, notif, own_login).await; // None means self-echo — mark read silently, no delivery. @@ -850,44 +801,12 @@ async fn poll_once( // deliberate: the hive-forge read-before-comment guard keys // off forge's own unread-state, and the agent reading the // thread via the CLI is what marks it read. Recorded only - // here in the Ok arm — a failed delivery hits the Err arm - // and `continue`s without recording, so it re-delivers next - // tick. + // here in the Ok arm — a failed delivery leaves the cursor + // untouched, so it re-delivers next tick. delivered.insert(id, updated_at); } Err(e) => { warn!(%id, error = ?e, "forge_notify: deliver failed — leaving unread"); - continue; - } - } - - // Auto-unsubscribe from broad repo watches after delivering a - // `subscribed` notification. Gated by HIVE_FORGE_KEEP_SUBSCRIPTIONS - // for triage / firehose agents (see - // `docs/forge.md::Auto-unsubscribe on broad watches`). - let reason = notif["reason"].as_str().unwrap_or(""); - if !keep_subscriptions - && reason == "subscribed" - && let Some(repo) = notif["repository"]["full_name"].as_str() - && !unsubbed_repos.contains(repo) - { - let unsub_url = format!("{forge_url}/api/v1/repos/{repo}/subscription"); - match client - .delete(&unsub_url) - .header("Authorization", format!("token {token}")) - .send() - .await - { - Ok(r) if r.status().is_success() || r.status().as_u16() == 404 => { - debug!(%repo, "forge_notify: unsubscribed from repo watch"); - unsubbed_repos.insert(repo.to_owned()); - } - Ok(r) => { - debug!(%repo, status = %r.status(), "forge_notify: unsub non-2xx (ignored)"); - } - Err(e) => { - debug!(%repo, error = ?e, "forge_notify: unsub request failed (ignored)"); - } } } } diff --git a/nix/templates/harness-base.nix b/nix/templates/harness-base.nix index 6af020f1..97348269 100644 --- a/nix/templates/harness-base.nix +++ b/nix/templates/harness-base.nix @@ -584,45 +584,6 @@ in ''; }; - options.hyperhive.forge.keepSubscriptions = lib.mkOption { - type = lib.types.bool; - default = true; - description = '' - When true (the default), the forge notification poller will NOT - auto-unsubscribe from repo watches after delivering a - "subscribed"-reason notification. Sub-agents keep their broad - subscriptions so they stay informed about repos they contribute to. - Set to false for agents (e.g. the manager) that use reason-based - filtering and do not need firehose-level repo visibility — they will - auto-unsubscribe after receiving a watched-repo notification. - ''; - }; - - options.hyperhive.forge.skipNotifyReasons = lib.mkOption { - type = lib.types.listOf lib.types.str; - default = [ ]; - example = [ - "subscribed" - "participating" - ]; - description = '' - Forgejo notification `reason` values to suppress in the forge - notification poller. Notifications with these reasons are marked - read and silently dropped; all others — including notifications - with a null or unrecognised reason — are delivered. - - Drop-list is safer than an allow-list: directed signals - (`review_requested`, `assigned`, `mention`) are never silently - missed even if Forgejo returns an unexpected reason string. - - Empty list (the default) delivers all notifications. Set to - `[ "subscribed" "participating" ]` for agents like the manager - that want only direct mentions and reviews, not the full repo - firehose. Rendered to the `HIVE_FORGE_NOTIFY_SKIP_REASONS` - environment variable consumed by the harness poller at runtime. - ''; - }; - options.hyperhive.dashboardLinks = lib.mkOption { type = lib.types.listOf ( lib.types.submodule { @@ -1155,12 +1116,6 @@ in # (compact-on-overflow) still fires when the session is truly full. HIVE_COMPACT_WATERMARK_TOKENS = "0"; } - // lib.optionalAttrs config.hyperhive.forge.keepSubscriptions { - HIVE_FORGE_KEEP_SUBSCRIPTIONS = "1"; - } - // lib.optionalAttrs (config.hyperhive.forge.skipNotifyReasons != [ ]) { - HIVE_FORGE_NOTIFY_SKIP_REASONS = lib.concatStringsSep "," config.hyperhive.forge.skipNotifyReasons; - } // lib.optionalAttrs (config.hyperhive._bashEnvFragments != "") { # Non-interactive bash invocations (claude's `Bash` tool runs # `bash -c`) source $BASH_ENV at startup — drops every active diff --git a/nix/templates/manager.nix b/nix/templates/manager.nix index b84b4bd1..370bd9e1 100644 --- a/nix/templates/manager.nix +++ b/nix/templates/manager.nix @@ -1,16 +1,7 @@ { ... }: { - imports = [ ./harness-base.nix ]; - # Entry-point for the privileged root agent (ruth). Referenced from # `flake.nix` (`nixosConfigurations.ruth`) and the meta-flake's - # `applied/ruth/flake.nix`. Forge subscription/participation firehose - # stays off so ruth's inbox isn't drowned in noise. - hyperhive.forge = { - keepSubscriptions = false; - skipNotifyReasons = [ - "subscribed" - "participating" - ]; - }; + # `applied/ruth/flake.nix`. + imports = [ ./harness-base.nix ]; }