fold forge_cursor into hyperhive-harness.json + prose comments (mara/argus review)
This commit is contained in:
parent
2549cc51ba
commit
f6d9ca7f99
4 changed files with 153 additions and 146 deletions
|
|
@ -7,11 +7,12 @@
|
|||
//! CLI is what marks it read. A delivery-dedupe cursor (thread id →
|
||||
//! last-delivered `updated_at`) stops the still-unread notification from
|
||||
//! re-firing a wake every poll; self-echo and drop-listed notifications
|
||||
//! are still marked read directly. The cursor is persisted to
|
||||
//! `$HYPERHIVE_STATE_DIR/forge-notify-cursor.json` (atomic tmp+rename) so
|
||||
//! a container rebuild/restart doesn't re-deliver the whole currently-
|
||||
//! unread backlog — the on-disk cursor is a private dedup mirror, NOT
|
||||
//! forge's read-state, so the read-before-comment guard is untouched.
|
||||
//! are still marked read directly. The cursor is persisted as the
|
||||
//! `forge_cursor` field of the harness's consolidated `hyperhive-harness.json`
|
||||
//! (via [`crate::events`]) and reloaded on boot so a container
|
||||
//! rebuild/restart doesn't re-deliver the whole currently-unread backlog —
|
||||
//! it is a private dedup mirror, NOT forge's read-state, so the
|
||||
//! read-before-comment guard is untouched.
|
||||
//!
|
||||
//! Activation gates, self-notification filtering, body excerpt +
|
||||
//! truncation + heading escape, wrapper formats (comment / review /
|
||||
|
|
@ -132,28 +133,21 @@ pub async fn run(socket: PathBuf) {
|
|||
// A new comment bumps `updated_at`, so the thread re-delivers. This is
|
||||
// purely anti-spam, NOT a correctness oracle.
|
||||
//
|
||||
// It is persisted to `$HYPERHIVE_STATE_DIR/forge-notify-cursor.json`
|
||||
// and reloaded here on boot so a container rebuild/restart doesn't
|
||||
// re-deliver the entire currently-unread backlog (#2106). Persisting
|
||||
// is safe because we only record a thread AFTER a successful broker
|
||||
// delivery, and the broker inbox is durable sqlite — so a persisted
|
||||
// "delivered" entry can never swallow a wake the agent never received.
|
||||
// The cursor file is a private dedup mirror, decoupled from forge's
|
||||
// own read-state, so it doesn't reintroduce the read-before-comment
|
||||
// coupling that the mark-read-on-delivery approach suffered.
|
||||
let cursor_path: Option<PathBuf> = if state_dir.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(PathBuf::from(format!(
|
||||
"{state_dir}/forge-notify-cursor.json"
|
||||
)))
|
||||
};
|
||||
let mut delivered: HashMap<u64, String> =
|
||||
cursor_path.as_deref().map(load_cursor).unwrap_or_default();
|
||||
// It is persisted as the `forge_cursor` field of the harness's
|
||||
// consolidated state file and reloaded here on boot so a container
|
||||
// rebuild/restart doesn't re-deliver the entire currently-unread
|
||||
// backlog. Persisting is safe because we only record a thread AFTER a
|
||||
// successful broker delivery, and the broker inbox is durable sqlite —
|
||||
// so a persisted "delivered" entry can never swallow a wake the agent
|
||||
// never received. The cursor is a private dedup mirror, decoupled from
|
||||
// forge's own read-state, so it doesn't reintroduce the
|
||||
// read-before-comment coupling that the mark-read-on-delivery approach
|
||||
// suffered.
|
||||
let mut delivered: HashMap<u64, String> = crate::events::read_forge_cursor();
|
||||
if !delivered.is_empty() {
|
||||
info!(
|
||||
entries = delivered.len(),
|
||||
"forge_notify: restored delivery-dedupe cursor from disk"
|
||||
"forge_notify: restored delivery-dedupe cursor"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -165,53 +159,12 @@ pub async fn run(socket: PathBuf) {
|
|||
&token,
|
||||
&socket,
|
||||
&mut delivered,
|
||||
cursor_path.as_deref(),
|
||||
&own_login,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
/// Load the persisted delivery-dedupe cursor from disk. Returns an empty
|
||||
/// map on any error — a missing file (first boot) or corrupt JSON both
|
||||
/// degrade to the pre-persistence behaviour (re-deliver the currently-
|
||||
/// unread set once) rather than aborting the poller.
|
||||
fn load_cursor(path: &Path) -> HashMap<u64, String> {
|
||||
match std::fs::read_to_string(path) {
|
||||
Ok(s) => serde_json::from_str(&s).unwrap_or_else(|e| {
|
||||
warn!(?path, error = %e, "forge_notify: cursor parse failed — starting empty");
|
||||
HashMap::new()
|
||||
}),
|
||||
Err(e) => {
|
||||
debug!(?path, error = %e, "forge_notify: no cursor file — starting empty");
|
||||
HashMap::new()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Persist the delivery-dedupe cursor atomically (write tmp + rename).
|
||||
/// Best-effort: logs on failure and lets the poll loop continue. Safe to
|
||||
/// call after a successful broker delivery because the wake is already
|
||||
/// durably queued in the broker sqlite inbox — recording "delivered" here
|
||||
/// can never swallow a notification the agent hasn't received.
|
||||
async fn persist_cursor(path: &Path, delivered: &HashMap<u64, String>) {
|
||||
let json = match serde_json::to_string(delivered) {
|
||||
Ok(j) => j,
|
||||
Err(e) => {
|
||||
warn!(?path, error = %e, "forge_notify: cursor serialize failed");
|
||||
return;
|
||||
}
|
||||
};
|
||||
let tmp = path.with_extension("json.tmp");
|
||||
if let Err(e) = tokio::fs::write(&tmp, json.as_bytes()).await {
|
||||
warn!(?tmp, error = %e, "forge_notify: cursor tmp write failed");
|
||||
return;
|
||||
}
|
||||
if let Err(e) = tokio::fs::rename(&tmp, path).await {
|
||||
warn!(?path, error = %e, "forge_notify: cursor rename failed");
|
||||
}
|
||||
}
|
||||
|
||||
/// Fetch a JSON value from a URL using the agent's forge token. Returns
|
||||
/// `None` on any HTTP or parse error (best-effort enrichment).
|
||||
async fn fetch_json(client: &reqwest::Client, url: &str, token: &str) -> Option<serde_json::Value> {
|
||||
|
|
@ -788,7 +741,6 @@ async fn poll_once(
|
|||
token: &str,
|
||||
socket: &Path,
|
||||
delivered: &mut HashMap<u64, String>,
|
||||
cursor_path: Option<&Path>,
|
||||
own_login: &str,
|
||||
) {
|
||||
let url = format!("{forge_url}/api/v1/notifications?all=false&limit=50");
|
||||
|
|
@ -899,10 +851,11 @@ async fn poll_once(
|
|||
cursor_dirty = true;
|
||||
}
|
||||
|
||||
// Flush the cursor to disk only when it changed, so a rebuild/restart
|
||||
// reloads it instead of re-delivering the whole unread backlog (#2106).
|
||||
if cursor_dirty && let Some(path) = cursor_path {
|
||||
persist_cursor(path, delivered).await;
|
||||
// Flush the cursor to the consolidated state file only when it
|
||||
// changed, so a rebuild/restart reloads it instead of re-delivering
|
||||
// the whole unread backlog.
|
||||
if cursor_dirty {
|
||||
crate::events::write_forge_cursor(delivered);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -976,53 +929,6 @@ mod tests {
|
|||
assert!(should_deliver(&delivered, 99, "2026-06-22T16:00:00Z"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cursor_serde_roundtrips_u64_keys() {
|
||||
// serde_json stringifies integer map keys; make sure the
|
||||
// round-trip preserves the u64 thread ids the cursor is keyed on.
|
||||
let mut delivered = HashMap::new();
|
||||
delivered.insert(42u64, "2026-06-22T16:00:00Z".to_owned());
|
||||
delivered.insert(99u64, "2026-06-22T17:30:00Z".to_owned());
|
||||
let json = serde_json::to_string(&delivered).unwrap();
|
||||
let back: HashMap<u64, String> = serde_json::from_str(&json).unwrap();
|
||||
assert_eq!(delivered, back);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_cursor_missing_file_is_empty() {
|
||||
// First boot: no cursor file yet → empty map (re-deliver once).
|
||||
let path = std::env::temp_dir().join("forge-notify-cursor-missing-xyzzy.json");
|
||||
let _ = std::fs::remove_file(&path);
|
||||
assert!(load_cursor(&path).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_cursor_corrupt_file_is_empty() {
|
||||
// A truncated / garbage cursor degrades to empty rather than
|
||||
// aborting the poller.
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"forge-notify-cursor-corrupt-{}.json",
|
||||
std::process::id()
|
||||
));
|
||||
std::fs::write(&path, "{not valid json").unwrap();
|
||||
assert!(load_cursor(&path).is_empty());
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_cursor_reads_written_map() {
|
||||
// The happy path: a persisted cursor reloads to the same map.
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"forge-notify-cursor-rw-{}.json",
|
||||
std::process::id()
|
||||
));
|
||||
let mut delivered = HashMap::new();
|
||||
delivered.insert(7u64, "2026-06-22T16:00:00Z".to_owned());
|
||||
std::fs::write(&path, serde_json::to_string(&delivered).unwrap()).unwrap();
|
||||
assert_eq!(load_cursor(&path), delivered);
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn escape_md_headings_escapes_top_level_atx() {
|
||||
// Argus reviews start with `## argus review`, which would
|
||||
|
|
|
|||
Loading…
Reference in a new issue