From daadb57963285320d2f6834fc36347c94d5af1fe Mon Sep 17 00:00:00 2001 From: wellkilo Date: Fri, 11 Sep 2026 00:20:27 +0800 Subject: [PATCH 01/72] perf(ecc2): stream dashboard output with a DB cursor Hydrate bounded session output snapshots at startup and recovery, then fetch only rows newer than the monotonic SQLite cursor during steady-state dashboard refreshes. Preserve cross-process visibility and bounded per-session caches, recover safely from transient database failures, and cover lifecycle, retry, and real child-process writes. --- ecc2/README.md | 6 + ecc2/src/session/output.rs | 10 -- ecc2/src/session/store.rs | 119 +++++++++++++ ecc2/src/tui/dashboard.rs | 355 ++++++++++++++++++++++++++++++++----- 4 files changed, 434 insertions(+), 56 deletions(-) diff --git a/ecc2/README.md b/ecc2/README.md index 71aad6da8..8f9cc6d80 100644 --- a/ecc2/README.md +++ b/ecc2/README.md @@ -14,6 +14,12 @@ It is usable as an alpha for local experimentation, but it is **not** the finish - worktree-aware session scaffolding - basic multi-session state and output tracking +Dashboard output is hydrated from SQLite at startup, explicit refresh, and +recovery, then synchronized with a monotonic database cursor. Because session +runners are separate processes, the database remains the cross-process source +of truth while steady-state refreshes read only rows appended since the previous +dashboard tick. + ## What This Is For ECC 2.0 is the layer above individual harness installs. diff --git a/ecc2/src/session/output.rs b/ecc2/src/session/output.rs index d7ac8745f..07aadf9d5 100644 --- a/ecc2/src/session/output.rs +++ b/ecc2/src/session/output.rs @@ -113,16 +113,6 @@ impl SessionOutputStore { }); } - pub fn replace_lines(&self, session_id: &str, lines: Vec) { - let mut buffer: VecDeque = lines.into_iter().collect(); - - while buffer.len() > self.capacity { - let _ = buffer.pop_front(); - } - - self.lock_buffers().insert(session_id.to_string(), buffer); - } - pub fn lines(&self, session_id: &str) -> Vec { self.lock_buffers() .get(session_id) diff --git a/ecc2/src/session/store.rs b/ecc2/src/session/store.rs index f71bb3640..3e77184a9 100644 --- a/ecc2/src/session/store.rs +++ b/ecc2/src/session/store.rs @@ -28,6 +28,30 @@ pub struct StateStore { conn: Connection, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct SessionOutputRecord { + pub id: i64, + pub session_id: String, + pub line: OutputLine, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct SessionOutputBatch { + pub cursor: i64, + pub records: Vec, +} + +fn output_record_from_row(row: &rusqlite::Row<'_>) -> rusqlite::Result { + let stream: String = row.get(2)?; + let text: String = row.get(3)?; + let timestamp: String = row.get(4)?; + Ok(SessionOutputRecord { + id: row.get(0)?, + session_id: row.get(1)?, + line: OutputLine::new(OutputStream::from_db_value(&stream), text, timestamp), + }) +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct HarnessAuditEntry { pub id: i64, @@ -4000,6 +4024,45 @@ impl StateStore { Ok(lines) } + pub(crate) fn get_output_snapshot( + &self, + limit_per_session: usize, + ) -> Result { + let limit_per_session = i64::try_from(limit_per_session.max(1)).unwrap_or(i64::MAX); + let mut stmt = self.conn.prepare( + "SELECT id, session_id, stream, line, timestamp + FROM ( + SELECT id, session_id, stream, line, timestamp, + ROW_NUMBER() OVER (PARTITION BY session_id ORDER BY id DESC) AS row_num + FROM session_output + ) + WHERE row_num <= ?1 + ORDER BY id ASC", + )?; + let records = stmt + .query_map(rusqlite::params![limit_per_session], output_record_from_row)? + .collect::, _>>()?; + let cursor = records.last().map(|record| record.id).unwrap_or(0); + + Ok(SessionOutputBatch { cursor, records }) + } + + pub(crate) fn get_output_since(&self, cursor: i64) -> Result { + let cursor = cursor.max(0); + let mut stmt = self.conn.prepare( + "SELECT id, session_id, stream, line, timestamp + FROM session_output + WHERE id > ?1 + ORDER BY id ASC", + )?; + let records = stmt + .query_map(rusqlite::params![cursor], output_record_from_row)? + .collect::, _>>()?; + let cursor = records.last().map(|record| record.id).unwrap_or(cursor); + + Ok(SessionOutputBatch { cursor, records }) + } + pub fn insert_tool_log( &self, session_id: &str, @@ -7382,6 +7445,62 @@ mod tests { Ok(()) } + #[test] + fn output_cursor_reads_a_bounded_snapshot_then_only_new_rows() -> Result<()> { + let tempdir = TestDir::new("store-output-cursor")?; + let db = StateStore::open(&tempdir.path().join("state.db"))?; + + db.insert_session(&build_session("session-1", SessionState::Running))?; + db.insert_session(&build_session("session-2", SessionState::Running))?; + db.append_output_line("session-1", OutputStream::Stdout, "one-a")?; + db.append_output_line("session-2", OutputStream::Stderr, "two-a")?; + db.append_output_line("session-1", OutputStream::Stdout, "one-b")?; + db.append_output_line("session-2", OutputStream::Stdout, "two-b")?; + db.append_output_line("session-1", OutputStream::Stdout, "one-c")?; + + let snapshot = db.get_output_snapshot(2)?; + assert_eq!(snapshot.cursor, 5); + assert_eq!( + snapshot + .records + .iter() + .map(|record| (record.session_id.as_str(), record.line.text.as_str())) + .collect::>(), + vec![ + ("session-2", "two-a"), + ("session-1", "one-b"), + ("session-2", "two-b"), + ("session-1", "one-c"), + ] + ); + + db.append_output_line("session-2", OutputStream::Stderr, "two-c")?; + let delta = db.get_output_since(snapshot.cursor)?; + assert_eq!(delta.cursor, 6); + assert_eq!(delta.records.len(), 1); + assert_eq!(delta.records[0].session_id, "session-2"); + assert_eq!(delta.records[0].line.text, "two-c"); + + let empty = db.get_output_since(delta.cursor)?; + assert_eq!(empty.cursor, delta.cursor); + assert!(empty.records.is_empty()); + + let query_plan = db + .conn + .prepare( + "EXPLAIN QUERY PLAN SELECT id FROM session_output WHERE id > ?1 ORDER BY id ASC", + )? + .query_map(rusqlite::params![snapshot.cursor], |row| { + row.get::<_, String>(3) + })? + .collect::, _>>()?; + assert!(query_plan + .iter() + .any(|detail| detail.contains("INTEGER PRIMARY KEY") && detail.contains("rowid>?"))); + + Ok(()) + } + #[test] fn message_round_trip_tracks_unread_counts_and_read_state() -> Result<()> { let tempdir = TestDir::new("store-messages")?; diff --git a/ecc2/src/tui/dashboard.rs b/ecc2/src/tui/dashboard.rs index c98b4e2c2..1a75f799a 100644 --- a/ecc2/src/tui/dashboard.rs +++ b/ecc2/src/tui/dashboard.rs @@ -10,7 +10,6 @@ use ratatui::{ use regex::Regex; use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; use std::time::UNIX_EPOCH; -use tokio::sync::broadcast; use super::widgets::{budget_state, format_currency, format_token_count, BudgetState, TokenMeter}; use crate::comms; @@ -18,13 +17,11 @@ use crate::config::{Config, PaneLayout, PaneNavigationAction, Theme}; use crate::notifications::{DesktopNotifier, NotificationEvent, WebhookNotifier}; use crate::observability::ToolLogEntry; use crate::session::manager; -use crate::session::output::{ - OutputEvent, OutputLine, OutputStream, SessionOutputStore, OUTPUT_BUFFER_LIMIT, -}; -use crate::session::store::{DaemonActivity, FileActivityOverlap, StateStore}; +use crate::session::output::{OutputLine, OutputStream, OUTPUT_BUFFER_LIMIT}; +use crate::session::store::{DaemonActivity, FileActivityOverlap, SessionOutputRecord, StateStore}; use crate::session::{ - ContextObservationPriority, DecisionLogEntry, FileActivityEntry, Session, SessionGrouping, - SessionBoardMeta, SessionHarnessInfo, SessionMessage, SessionState, + ContextObservationPriority, DecisionLogEntry, FileActivityEntry, Session, SessionBoardMeta, + SessionGrouping, SessionHarnessInfo, SessionMessage, SessionState, }; use crate::worktree; @@ -79,16 +76,38 @@ struct TestRunSummary { passed: usize, } +fn append_output_records( + cache: &mut HashMap>, + records: Vec, +) { + let mut touched_sessions = HashSet::new(); + for record in records { + cache + .entry(record.session_id.clone()) + .or_default() + .push(record.line); + touched_sessions.insert(record.session_id); + } + + for session_id in touched_sessions { + if let Some(lines) = cache.get_mut(&session_id) { + let overflow = lines.len().saturating_sub(OUTPUT_BUFFER_LIMIT); + if overflow > 0 { + lines.drain(..overflow); + } + } + } +} + pub struct Dashboard { db: StateStore, cfg: Config, - output_store: SessionOutputStore, - output_rx: broadcast::Receiver, notifier: DesktopNotifier, webhook_notifier: WebhookNotifier, sessions: Vec, session_harnesses: HashMap, session_output_cache: HashMap>, + output_cursor: Option, unread_message_counts: HashMap, approval_queue_counts: HashMap, approval_queue_preview: Vec, @@ -503,14 +522,6 @@ fn load_session_harnesses( impl Dashboard { pub fn new(db: StateStore, cfg: Config) -> Self { - Self::with_output_store(db, cfg, SessionOutputStore::default()) - } - - pub fn with_output_store( - db: StateStore, - cfg: Config, - output_store: SessionOutputStore, - ) -> Self { let pane_size_percent = configured_pane_size(&cfg, cfg.pane_layout); let initial_cost_metrics_signature = metrics_file_signature(&cfg.cost_metrics_path()); let initial_tool_activity_signature = @@ -533,7 +544,6 @@ impl Dashboard { .ok() .flatten() .map(|message| message.id); - let output_rx = output_store.subscribe(); let notifier = DesktopNotifier::new(cfg.desktop_notifications.clone()); let webhook_notifier = WebhookNotifier::new(cfg.webhook_notifications.clone()); let mut session_table_state = TableState::default(); @@ -544,13 +554,12 @@ impl Dashboard { let mut dashboard = Self { db, cfg, - output_store, - output_rx, notifier, webhook_notifier, sessions, session_harnesses, session_output_cache: HashMap::new(), + output_cursor: None, unread_message_counts: HashMap::new(), approval_queue_counts: HashMap::new(), approval_queue_preview: Vec::new(), @@ -624,6 +633,7 @@ impl Dashboard { dashboard.sync_handoff_backlog_counts(); dashboard.sync_board_meta(); dashboard.sync_global_handoff_backlog(); + dashboard.sync_output_cache(); dashboard.sync_selected_output(); dashboard.sync_selected_diff(); dashboard.sync_selected_messages(); @@ -3212,6 +3222,7 @@ impl Dashboard { } pub fn refresh(&mut self) { + self.output_cursor = None; self.sync_from_store(); } @@ -3993,15 +4004,6 @@ impl Dashboard { } pub async fn tick(&mut self) { - loop { - match self.output_rx.try_recv() { - Ok(_event) => {} - Err(broadcast::error::TryRecvError::Empty) => break, - Err(broadcast::error::TryRecvError::Lagged(_)) => continue, - Err(broadcast::error::TryRecvError::Closed) => break, - } - } - if let Err(error) = manager::activate_pending_worktree_sessions(&self.db, &self.cfg).await { tracing::warn!("Failed to activate queued worktree sessions: {error}"); } @@ -4077,14 +4079,17 @@ impl Dashboard { let (heartbeat_enforcement, budget_enforcement, conflict_enforcement) = self.sync_runtime_metrics(); let selected_id = self.selected_session_id().map(ToOwned::to_owned); - self.sessions = match self.db.list_sessions() { + let sessions_refreshed = match self.db.list_sessions() { Ok(mut sessions) => { sort_sessions_for_display(&mut sessions); - sessions + self.sessions = sessions; + true } Err(error) => { tracing::warn!("Failed to refresh sessions: {error}"); - Vec::new() + self.output_cursor = None; + self.sessions.clear(); + false } }; self.session_harnesses = load_session_harnesses(&self.db, &self.cfg, &self.sessions); @@ -4103,7 +4108,9 @@ impl Dashboard { self.sync_approval_notifications(); self.sync_global_handoff_backlog(); self.sync_daemon_activity(); - self.sync_output_cache(); + if sessions_refreshed { + self.sync_output_cache(); + } self.sync_selection_by_id(selected_id.as_deref()); self.ensure_selected_pane_visible(); self.sync_selected_output(); @@ -4489,17 +4496,24 @@ impl Dashboard { self.session_output_cache .retain(|session_id, _| active_session_ids.contains(session_id.as_str())); - for session in &self.sessions { - match self.db.get_output_lines(&session.id, OUTPUT_BUFFER_LIMIT) { - Ok(lines) => { - self.output_store.replace_lines(&session.id, lines.clone()); - self.session_output_cache.insert(session.id.clone(), lines); - } - Err(error) => { - tracing::warn!("Failed to load session output for {}: {error}", session.id); - } + let batch = match self.output_cursor { + Some(cursor) => self.db.get_output_since(cursor), + None => self.db.get_output_snapshot(OUTPUT_BUFFER_LIMIT), + }; + let batch = match batch { + Ok(batch) => batch, + Err(error) => { + tracing::warn!("Failed to refresh session output cache: {error}"); + return; } + }; + + if self.output_cursor.is_none() { + self.session_output_cache.clear(); } + self.output_cursor = Some(batch.cursor); + + append_output_records(&mut self.session_output_cache, batch.records); } fn ensure_selected_pane_visible(&mut self) { @@ -13147,6 +13161,258 @@ diff --git a/src/lib.rs b/src/lib.rs Ok(()) } + #[test] + fn output_cache_appends_rows_written_by_another_process_without_rehydrating() -> Result<()> { + let db_path = + std::env::temp_dir().join(format!("ecc2-output-cursor-{}.db", Uuid::new_v4())); + let db = StateStore::open(&db_path)?; + let session = sample_session("session-1", "claude", SessionState::Running, None, 0, 0); + db.insert_session(&session)?; + db.append_output_line("session-1", OutputStream::Stdout, "persisted-before-open")?; + + let mut dashboard = Dashboard::new(db, Config::default()); + assert!(dashboard + .selected_output_text() + .contains("persisted-before-open")); + dashboard + .session_output_cache + .entry("session-1".to_string()) + .or_default() + .push(test_output_line(OutputStream::Stdout, "cache-only")); + + let child = Command::new(std::env::current_exe()?) + .args([ + "--exact", + "tui::dashboard::tests::output_cursor_child_writer", + "--ignored", + "--nocapture", + ]) + .env("ECC2_OUTPUT_CURSOR_CHILD_DB", &db_path) + .status()?; + assert!(child.success(), "child output writer should succeed"); + dashboard.sync_output_cache(); + + let text = dashboard.selected_output_text(); + assert!(text.contains("persisted-before-open")); + assert!(text.contains("cache-only")); + assert!(text.contains("persisted-after-open")); + + dashboard.sync_output_cache(); + assert_eq!( + dashboard + .selected_output_lines() + .iter() + .filter(|line| line.text == "persisted-after-open") + .count(), + 1 + ); + + let _ = std::fs::remove_file(db_path); + Ok(()) + } + + #[test] + #[ignore = "helper invoked by output cursor cross-process test"] + fn output_cursor_child_writer() -> Result<()> { + let Some(db_path) = std::env::var_os("ECC2_OUTPUT_CURSOR_CHILD_DB") else { + return Ok(()); + }; + StateStore::open(Path::new(&db_path))?.append_output_line( + "session-1", + OutputStream::Stderr, + "persisted-after-open", + ) + } + + #[test] + fn output_cache_rehydrates_after_transient_session_list_failure() -> Result<()> { + let db_path = + std::env::temp_dir().join(format!("ecc2-output-recovery-{}.db", Uuid::new_v4())); + let db = StateStore::open(&db_path)?; + let session = sample_session("session-1", "claude", SessionState::Running, None, 0, 0); + db.insert_session(&session)?; + db.append_output_line("session-1", OutputStream::Stdout, "persisted-output")?; + + let mut dashboard = Dashboard::new(db, Config::default()); + assert!(dashboard + .selected_output_text() + .contains("persisted-output")); + dashboard + .session_output_cache + .entry("session-1".to_string()) + .or_default() + .push(test_output_line(OutputStream::Stdout, "cache-only")); + + let schema = rusqlite::Connection::open(&db_path)?; + schema.execute("ALTER TABLE sessions RENAME TO unavailable_sessions", [])?; + dashboard.sync_from_store(); + assert!(dashboard.sessions.is_empty()); + assert!(dashboard.session_output_cache["session-1"] + .iter() + .any(|line| line.text == "cache-only")); + assert!(dashboard.output_cursor.is_none()); + + dashboard.sync_from_store(); + assert!(dashboard.session_output_cache["session-1"] + .iter() + .any(|line| line.text == "cache-only")); + assert!(dashboard.output_cursor.is_none()); + + schema.execute("ALTER TABLE unavailable_sessions RENAME TO sessions", [])?; + dashboard.sync_from_store(); + + assert_eq!(dashboard.sessions.len(), 1); + assert!(dashboard + .selected_output_text() + .contains("persisted-output")); + assert!(!dashboard.selected_output_text().contains("cache-only")); + + let _ = std::fs::remove_file(db_path); + Ok(()) + } + + #[test] + fn output_cache_tracks_session_add_delete_and_same_id_recreation() -> Result<()> { + let db_path = + std::env::temp_dir().join(format!("ecc2-output-lifecycle-{}.db", Uuid::new_v4())); + let db = StateStore::open(&db_path)?; + db.insert_session(&sample_session( + "session-1", + "claude", + SessionState::Running, + None, + 0, + 0, + ))?; + db.append_output_line("session-1", OutputStream::Stdout, "first-session")?; + + let mut dashboard = Dashboard::new(db, Config::default()); + let external = StateStore::open(&db_path)?; + external.insert_session(&sample_session( + "session-2", + "codex", + SessionState::Running, + None, + 0, + 0, + ))?; + external.append_output_line("session-2", OutputStream::Stderr, "new-session")?; + dashboard.sync_from_store(); + + assert!(dashboard.sessions.iter().any(|session| session.id == "session-2")); + assert_eq!(dashboard.session_output_cache["session-2"][0].text, "new-session"); + + external.delete_session("session-2")?; + dashboard.sync_from_store(); + assert!(!dashboard.session_output_cache.contains_key("session-2")); + + external.insert_session(&sample_session( + "session-2", + "codex", + SessionState::Running, + None, + 0, + 0, + ))?; + external.append_output_line("session-2", OutputStream::Stdout, "replacement-session")?; + dashboard.sync_from_store(); + + let replacement = &dashboard.session_output_cache["session-2"]; + assert_eq!(replacement.len(), 1); + assert_eq!(replacement[0].text, "replacement-session"); + + let _ = std::fs::remove_file(db_path); + Ok(()) + } + + #[test] + fn output_cache_retries_delta_after_transient_output_query_failure() -> Result<()> { + let db_path = + std::env::temp_dir().join(format!("ecc2-output-query-retry-{}.db", Uuid::new_v4())); + let db = StateStore::open(&db_path)?; + db.insert_session(&sample_session( + "session-1", + "claude", + SessionState::Running, + None, + 0, + 0, + ))?; + db.append_output_line("session-1", OutputStream::Stdout, "persisted-before")?; + + let mut dashboard = Dashboard::new(db, Config::default()); + dashboard + .session_output_cache + .get_mut("session-1") + .expect("hydrated output") + .push(test_output_line(OutputStream::Stdout, "cache-only")); + let cursor = dashboard.output_cursor; + + let schema = rusqlite::Connection::open(&db_path)?; + schema.execute( + "ALTER TABLE session_output RENAME TO unavailable_session_output", + [], + )?; + dashboard.sync_output_cache(); + assert_eq!(dashboard.output_cursor, cursor); + assert!(dashboard.session_output_cache["session-1"] + .iter() + .any(|line| line.text == "cache-only")); + + schema.execute( + "ALTER TABLE unavailable_session_output RENAME TO session_output", + [], + )?; + StateStore::open(&db_path)?.append_output_line( + "session-1", + OutputStream::Stderr, + "persisted-after", + )?; + dashboard.sync_output_cache(); + + let output = &dashboard.session_output_cache["session-1"]; + assert!(output.iter().any(|line| line.text == "cache-only")); + assert_eq!( + output + .iter() + .filter(|line| line.text == "persisted-after") + .count(), + 1 + ); + + let _ = std::fs::remove_file(db_path); + Ok(()) + } + + #[test] + fn append_output_records_bounds_each_session_to_the_latest_window() { + let mut cache = HashMap::from([( + "session-2".to_string(), + vec![test_output_line(OutputStream::Stderr, "other-session")], + )]); + let records = (0..(OUTPUT_BUFFER_LIMIT + 5)) + .map(|index| crate::session::store::SessionOutputRecord { + id: index as i64 + 1, + session_id: "session-1".to_string(), + line: test_output_line(OutputStream::Stdout, &format!("line-{index}")), + }) + .collect(); + + append_output_records(&mut cache, records); + + let session_lines = cache.get("session-1").expect("session output"); + assert_eq!(session_lines.len(), OUTPUT_BUFFER_LIMIT); + assert_eq!( + session_lines.first().map(|line| line.text.as_str()), + Some("line-5") + ); + assert_eq!( + session_lines.last().map(|line| line.text.as_str()), + Some(format!("line-{}", OUTPUT_BUFFER_LIMIT + 4).as_str()) + ); + assert_eq!(cache["session-2"][0].text, "other-session"); + } + #[test] fn submit_search_tracks_matches_and_sets_navigation_note() { let mut dashboard = test_dashboard( @@ -14917,8 +15183,6 @@ diff --git a/src/lib.rs b/src/lib.rs ) }) .collect(); - let output_store = SessionOutputStore::default(); - let output_rx = output_store.subscribe(); let mut session_table_state = TableState::default(); if !sessions.is_empty() { session_table_state.select(Some(selected_session)); @@ -14928,13 +15192,12 @@ diff --git a/src/lib.rs b/src/lib.rs db: StateStore::open(Path::new(":memory:")).expect("open test db"), pane_size_percent: configured_pane_size(&cfg, cfg.pane_layout), cfg, - output_store, - output_rx, notifier, webhook_notifier, sessions, session_harnesses, session_output_cache: HashMap::new(), + output_cursor: None, unread_message_counts: HashMap::new(), approval_queue_counts: HashMap::new(), approval_queue_preview: Vec::new(), From 8cd852136f302c6b9dd88ff3eea11050da9e17a5 Mon Sep 17 00:00:00 2001 From: wellkilo Date: Fri, 11 Sep 2026 01:03:17 +0800 Subject: [PATCH 02/72] fix(ecc2): bound output cursor recovery Page dashboard deltas, preserve incremental refreshes, isolate reused session IDs by creation time, and use ownership-based cache updates. Add cross-process, lifecycle, and retry coverage for the reviewed edge cases. --- ecc2/README.md | 8 ++-- ecc2/src/session/output.rs | 2 + ecc2/src/session/store.rs | 28 ++++++++++--- ecc2/src/tui/dashboard.rs | 84 ++++++++++++++++++++++++++------------ 4 files changed, 86 insertions(+), 36 deletions(-) diff --git a/ecc2/README.md b/ecc2/README.md index 8f9cc6d80..2ea06c961 100644 --- a/ecc2/README.md +++ b/ecc2/README.md @@ -14,10 +14,10 @@ It is usable as an alpha for local experimentation, but it is **not** the finish - worktree-aware session scaffolding - basic multi-session state and output tracking -Dashboard output is hydrated from SQLite at startup, explicit refresh, and -recovery, then synchronized with a monotonic database cursor. Because session -runners are separate processes, the database remains the cross-process source -of truth while steady-state refreshes read only rows appended since the previous +Dashboard output is hydrated from SQLite at startup and after recovery, then +synchronized with a monotonic database cursor. Because session runners are +separate processes, the database remains the cross-process source of truth +while steady-state refreshes read only the rows appended since the previous dashboard tick. ## What This Is For diff --git a/ecc2/src/session/output.rs b/ecc2/src/session/output.rs index 07aadf9d5..1edd3f800 100644 --- a/ecc2/src/session/output.rs +++ b/ecc2/src/session/output.rs @@ -5,6 +5,8 @@ use serde::{Deserialize, Serialize}; use tokio::sync::broadcast; pub const OUTPUT_BUFFER_LIMIT: usize = 1000; +/// Maximum number of cross-process output rows applied during one dashboard refresh. +pub const OUTPUT_DELTA_BATCH_LIMIT: usize = 4096; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum OutputStream { diff --git a/ecc2/src/session/store.rs b/ecc2/src/session/store.rs index 3e77184a9..de1af81fc 100644 --- a/ecc2/src/session/store.rs +++ b/ecc2/src/session/store.rs @@ -41,6 +41,7 @@ pub(crate) struct SessionOutputBatch { pub records: Vec, } +/// Converts one persisted output row into the dashboard's typed record. fn output_record_from_row(row: &rusqlite::Row<'_>) -> rusqlite::Result { let stream: String = row.get(2)?; let text: String = row.get(3)?; @@ -4024,6 +4025,7 @@ impl StateStore { Ok(lines) } + /// Returns a bounded recent-output snapshot and its highest persisted row ID. pub(crate) fn get_output_snapshot( &self, limit_per_session: usize, @@ -4047,16 +4049,23 @@ impl StateStore { Ok(SessionOutputBatch { cursor, records }) } - pub(crate) fn get_output_since(&self, cursor: i64) -> Result { + /// Returns at most `limit` output rows newer than `cursor` in insertion order. + pub(crate) fn get_output_since( + &self, + cursor: i64, + limit: usize, + ) -> Result { let cursor = cursor.max(0); + let limit = i64::try_from(limit.max(1)).unwrap_or(i64::MAX); let mut stmt = self.conn.prepare( "SELECT id, session_id, stream, line, timestamp FROM session_output WHERE id > ?1 - ORDER BY id ASC", + ORDER BY id ASC + LIMIT ?2", )?; let records = stmt - .query_map(rusqlite::params![cursor], output_record_from_row)? + .query_map(rusqlite::params![cursor, limit], output_record_from_row)? .collect::, _>>()?; let cursor = records.last().map(|record| record.id).unwrap_or(cursor); @@ -7475,14 +7484,21 @@ mod tests { ); db.append_output_line("session-2", OutputStream::Stderr, "two-c")?; - let delta = db.get_output_since(snapshot.cursor)?; + db.append_output_line("session-1", OutputStream::Stdout, "one-d")?; + let delta = db.get_output_since(snapshot.cursor, 1)?; assert_eq!(delta.cursor, 6); assert_eq!(delta.records.len(), 1); assert_eq!(delta.records[0].session_id, "session-2"); assert_eq!(delta.records[0].line.text, "two-c"); - let empty = db.get_output_since(delta.cursor)?; - assert_eq!(empty.cursor, delta.cursor); + let next = db.get_output_since(delta.cursor, 1)?; + assert_eq!(next.cursor, 7); + assert_eq!(next.records.len(), 1); + assert_eq!(next.records[0].session_id, "session-1"); + assert_eq!(next.records[0].line.text, "one-d"); + + let empty = db.get_output_since(next.cursor, 1)?; + assert_eq!(empty.cursor, next.cursor); assert!(empty.records.is_empty()); let query_plan = db diff --git a/ecc2/src/tui/dashboard.rs b/ecc2/src/tui/dashboard.rs index 1a75f799a..deb34605a 100644 --- a/ecc2/src/tui/dashboard.rs +++ b/ecc2/src/tui/dashboard.rs @@ -17,7 +17,9 @@ use crate::config::{Config, PaneLayout, PaneNavigationAction, Theme}; use crate::notifications::{DesktopNotifier, NotificationEvent, WebhookNotifier}; use crate::observability::ToolLogEntry; use crate::session::manager; -use crate::session::output::{OutputLine, OutputStream, OUTPUT_BUFFER_LIMIT}; +use crate::session::output::{ + OutputLine, OutputStream, OUTPUT_BUFFER_LIMIT, OUTPUT_DELTA_BATCH_LIMIT, +}; use crate::session::store::{DaemonActivity, FileActivityOverlap, SessionOutputRecord, StateStore}; use crate::session::{ ContextObservationPriority, DecisionLogEntry, FileActivityEntry, Session, SessionBoardMeta, @@ -76,10 +78,11 @@ struct TestRunSummary { passed: usize, } +/// Consumes an output cache and returns a new bounded cache with `records` appended. fn append_output_records( - cache: &mut HashMap>, + mut cache: HashMap>, records: Vec, -) { +) -> HashMap> { let mut touched_sessions = HashSet::new(); for record in records { cache @@ -97,6 +100,8 @@ fn append_output_records( } } } + + cache } pub struct Dashboard { @@ -107,6 +112,7 @@ pub struct Dashboard { sessions: Vec, session_harnesses: HashMap, session_output_cache: HashMap>, + session_output_generations: HashMap>, output_cursor: Option, unread_message_counts: HashMap, approval_queue_counts: HashMap, @@ -521,6 +527,7 @@ fn load_session_harnesses( } impl Dashboard { + /// Builds the dashboard and hydrates its initial bounded output snapshot. pub fn new(db: StateStore, cfg: Config) -> Self { let pane_size_percent = configured_pane_size(&cfg, cfg.pane_layout); let initial_cost_metrics_signature = metrics_file_signature(&cfg.cost_metrics_path()); @@ -539,6 +546,10 @@ impl Dashboard { .iter() .map(|session| (session.id.clone(), session.state.clone())) .collect(); + let session_output_generations = sessions + .iter() + .map(|session| (session.id.clone(), session.created_at)) + .collect(); let initial_approval_message_id = db .latest_unread_approval_message() .ok() @@ -559,6 +570,7 @@ impl Dashboard { sessions, session_harnesses, session_output_cache: HashMap::new(), + session_output_generations, output_cursor: None, unread_message_counts: HashMap::new(), approval_queue_counts: HashMap::new(), @@ -3221,8 +3233,8 @@ impl Dashboard { )); } + /// Refreshes persisted dashboard state while preserving the output cursor. pub fn refresh(&mut self) { - self.output_cursor = None; self.sync_from_store(); } @@ -4075,6 +4087,7 @@ impl Dashboard { ) } + /// Synchronizes dashboard state, deferring output recovery until sessions load. fn sync_from_store(&mut self) { let (heartbeat_enforcement, budget_enforcement, conflict_enforcement) = self.sync_runtime_metrics(); @@ -4488,16 +4501,24 @@ impl Dashboard { } fn sync_output_cache(&mut self) { - let active_session_ids: HashSet<_> = self + let active_session_generations: HashMap<_, _> = self .sessions .iter() - .map(|session| session.id.as_str()) + .map(|session| (session.id.clone(), session.created_at)) .collect(); - self.session_output_cache - .retain(|session_id, _| active_session_ids.contains(session_id.as_str())); + let cached_generations = &self.session_output_generations; + self.session_output_cache = std::mem::take(&mut self.session_output_cache) + .into_iter() + .filter(|(session_id, _)| { + active_session_generations.get(session_id) == cached_generations.get(session_id) + }) + .collect(); + self.session_output_generations = active_session_generations; let batch = match self.output_cursor { - Some(cursor) => self.db.get_output_since(cursor), + Some(cursor) => self + .db + .get_output_since(cursor, OUTPUT_DELTA_BATCH_LIMIT), None => self.db.get_output_snapshot(OUTPUT_BUFFER_LIMIT), }; let batch = match batch { @@ -4509,11 +4530,14 @@ impl Dashboard { }; if self.output_cursor.is_none() { - self.session_output_cache.clear(); + self.session_output_cache = HashMap::new(); } self.output_cursor = Some(batch.cursor); - append_output_records(&mut self.session_output_cache, batch.records); + self.session_output_cache = append_output_records( + std::mem::take(&mut self.session_output_cache), + batch.records, + ); } fn ensure_selected_pane_visible(&mut self) { @@ -5226,6 +5250,7 @@ impl Dashboard { .map(|session| session.id.as_str()) } + /// Returns the selected session's currently cached output window. fn selected_output_lines(&self) -> &[OutputLine] { self.selected_session_id() .and_then(|session_id| self.session_output_cache.get(session_id)) @@ -13190,7 +13215,7 @@ diff --git a/src/lib.rs b/src/lib.rs .env("ECC2_OUTPUT_CURSOR_CHILD_DB", &db_path) .status()?; assert!(child.success(), "child output writer should succeed"); - dashboard.sync_output_cache(); + dashboard.refresh(); let text = dashboard.selected_output_text(); assert!(text.contains("persisted-before-open")); @@ -13299,21 +13324,23 @@ diff --git a/src/lib.rs b/src/lib.rs external.append_output_line("session-2", OutputStream::Stderr, "new-session")?; dashboard.sync_from_store(); - assert!(dashboard.sessions.iter().any(|session| session.id == "session-2")); - assert_eq!(dashboard.session_output_cache["session-2"][0].text, "new-session"); + assert!(dashboard + .sessions + .iter() + .any(|session| session.id == "session-2")); + assert_eq!( + dashboard.session_output_cache["session-2"][0].text, + "new-session" + ); external.delete_session("session-2")?; - dashboard.sync_from_store(); - assert!(!dashboard.session_output_cache.contains_key("session-2")); - - external.insert_session(&sample_session( - "session-2", - "codex", - SessionState::Running, - None, - 0, - 0, - ))?; + let replacement_time = Utc::now() + chrono::Duration::seconds(1); + external.insert_session(&Session { + created_at: replacement_time, + updated_at: replacement_time, + last_heartbeat_at: replacement_time, + ..sample_session("session-2", "codex", SessionState::Running, None, 0, 0) + })?; external.append_output_line("session-2", OutputStream::Stdout, "replacement-session")?; dashboard.sync_from_store(); @@ -13398,7 +13425,7 @@ diff --git a/src/lib.rs b/src/lib.rs }) .collect(); - append_output_records(&mut cache, records); + cache = append_output_records(cache, records); let session_lines = cache.get("session-1").expect("session output"); assert_eq!(session_lines.len(), OUTPUT_BUFFER_LIMIT); @@ -15183,6 +15210,10 @@ diff --git a/src/lib.rs b/src/lib.rs ) }) .collect(); + let session_output_generations = sessions + .iter() + .map(|session| (session.id.clone(), session.created_at)) + .collect(); let mut session_table_state = TableState::default(); if !sessions.is_empty() { session_table_state.select(Some(selected_session)); @@ -15197,6 +15228,7 @@ diff --git a/src/lib.rs b/src/lib.rs sessions, session_harnesses, session_output_cache: HashMap::new(), + session_output_generations, output_cursor: None, unread_message_counts: HashMap::new(), approval_queue_counts: HashMap::new(), From 3033436dccd72f8fee14473986d48aa898a8470f Mon Sep 17 00:00:00 2001 From: Wu Shuwen Date: Sat, 12 Sep 2026 08:46:04 +0800 Subject: [PATCH 04/72] fix: filter epic sync issues by label (#3089) github-coordination sync listed every repo issue and pushed the epic label onto all of them (#3084). Scope the listing to issues carrying the policy's epic label plus issues whose body still holds the coordination marker (label-drift recovery), deduped by number, and reject an empty labels.epic in loadPolicy. Tests cover the filtered path and the recovery path with exact gh argv. Independent exact-head review passed with no P0/P1; CI 44/44 at the head. --- scripts/lib/github-coordination/actions.js | 19 ++++- scripts/lib/github-coordination/gh-api.js | 7 +- scripts/lib/github-coordination/policy.js | 6 +- tests/lib/github-coordination-policy.test.js | 7 ++ tests/scripts/github-coordination.test.js | 81 ++++++++++++++++++++ 5 files changed, 116 insertions(+), 4 deletions(-) diff --git a/scripts/lib/github-coordination/actions.js b/scripts/lib/github-coordination/actions.js index 06cd0d383..b9a3104ae 100644 --- a/scripts/lib/github-coordination/actions.js +++ b/scripts/lib/github-coordination/actions.js @@ -84,7 +84,24 @@ function applySync(repo, options = {}, context = {}) { assertValidRepo(repo); const policy = context.policy || loadPolicy(context.rootDir || process.cwd(), options.configPath); const store = context.store || null; - const issues = listIssues(repo, { ...options, state: options.state || 'all', limit: options.limit || 100 }); + const labelIssues = listIssues(repo, { + ...options, + state: options.state || 'all', + limit: options.limit || 100, + label: policy.labels && policy.labels.epic, + }); + const marker = policy.sectionMarker || 'ecc-coordination'; + const coordinatedIssues = listIssues(repo, { + ...options, + state: options.state || 'all', + limit: options.limit || 100, + search: `in:body "${marker}:start"`, + }); + const issues = Array.from( + new Map( + [...labelIssues, ...coordinatedIssues].map(issue => [String(issue.number), issue]) + ).values() + ); const syncedAt = new Date().toISOString(); const results = []; diff --git a/scripts/lib/github-coordination/gh-api.js b/scripts/lib/github-coordination/gh-api.js index d7669cdb7..dd2cb4397 100644 --- a/scripts/lib/github-coordination/gh-api.js +++ b/scripts/lib/github-coordination/gh-api.js @@ -102,7 +102,7 @@ function listIssues(repo, options = {}) { const { owner, name } = normalizeRepo(repo); const limit = Number.isFinite(options.limit) ? options.limit : 100; const state = options.state || 'all'; - return runGhJson([ + const args = [ 'issue', 'list', '--repo', @@ -111,9 +111,12 @@ function listIssues(repo, options = {}) { state, '--limit', String(limit), + ...(options.label ? ['--label', options.label] : []), + ...(options.search ? ['--search', options.search] : []), '--json', 'number,title,body,url,state,labels,author,updatedAt,assignees', - ], options) || []; + ]; + return runGhJson(args, options) || []; } function editIssue(repo, issueNumber, options = {}) { diff --git a/scripts/lib/github-coordination/policy.js b/scripts/lib/github-coordination/policy.js index dc0b0d549..6d93e88b2 100644 --- a/scripts/lib/github-coordination/policy.js +++ b/scripts/lib/github-coordination/policy.js @@ -74,10 +74,14 @@ function loadPolicy(rootDir = process.cwd(), configPath = null) { const branchModel = typeof parsed.branchModel === 'object' && parsed.branchModel !== null && !Array.isArray(parsed.branchModel) ? parsed.branchModel : {}; const project = typeof parsed.project === 'object' && parsed.project !== null && !Array.isArray(parsed.project) ? parsed.project : {}; const fieldNames = typeof project.fieldNames === 'object' && project.fieldNames !== null && !Array.isArray(project.fieldNames) ? project.fieldNames : {}; + const mergedLabels = { ...DEFAULT_LABELS, ...labels }; + if (typeof mergedLabels.epic !== 'string' || !mergedLabels.epic.trim()) { + throw new Error(`Policy file ${resolvedPath} must define labels.epic as a non-empty string`); + } return { ...DEFAULT_POLICY, ...parsed, - labels: { ...DEFAULT_LABELS, ...labels }, + labels: mergedLabels, review: { ...DEFAULT_POLICY.review, ...review }, validation: { ...DEFAULT_POLICY.validation, ...validation }, branchModel: { ...DEFAULT_POLICY.branchModel, ...branchModel }, diff --git a/tests/lib/github-coordination-policy.test.js b/tests/lib/github-coordination-policy.test.js index 0420789ef..352e537b0 100644 --- a/tests/lib/github-coordination-policy.test.js +++ b/tests/lib/github-coordination-policy.test.js @@ -128,6 +128,13 @@ if (test('merges labels when parsed.labels is a plain object', () => { }); })) passed++; else failed++; +if (test('rejects an empty epic label', () => { + withTempDir(tmpDir => { + writeConfig(tmpDir, { labels: { epic: ' ' } }); + assert.throws(() => loadPolicy(tmpDir), /labels\.epic.*non-empty string/); + }); +})) passed++; else failed++; + if (test('falls back to empty labels when parsed.labels is null', () => { withTempDir(tmpDir => { writeConfig(tmpDir, { labels: null }); diff --git a/tests/scripts/github-coordination.test.js b/tests/scripts/github-coordination.test.js index 158fc7c91..73394a1ef 100644 --- a/tests/scripts/github-coordination.test.js +++ b/tests/scripts/github-coordination.test.js @@ -243,6 +243,87 @@ async function runTests() { passed++; else failed++; + if ( + await test('sync filters the issue list to the configured epic label', async () => { + const rootDir = createTempDir('github-coordination-sync-'); + const dbPath = path.join(rootDir, 'state.db'); + + try { + const epicIssue = { + number: 12, + title: 'Ship GitHub-native coordination', + body: '# Ship GitHub-native coordination', + url: 'https://github.com/affaan-m/ECC/issues/12', + state: 'OPEN', + labels: [{ name: 'epic' }], + author: { login: 'maintainer' }, + updatedAt: '2026-06-01T12:00:00Z' + }; + const shim = writeGhShim(rootDir, { + 'issue list --repo affaan-m/ECC --state all --limit 100 --label epic --json number,title,body,url,state,labels,author,updatedAt,assignees': [epicIssue], + 'issue list --repo affaan-m/ECC --state all --limit 100 --search in:body "ecc-coordination:start" --json number,title,body,url,state,labels,author,updatedAt,assignees': [] + }); + + const result = run(['sync', '--repo', 'affaan-m/ECC', '--db', dbPath, '--dry-run', '--json'], { + cwd: rootDir, + env: { + ECC_GH_SHIM: shim.shimPath, + ECC_GH_SHIM_LOG: shim.logPath + } + }); + assert.strictEqual(result.status, 0, result.stderr); + const payload = parseJson(result.stdout); + assert.strictEqual(payload.count, 1); + assert.strictEqual(payload.items[0].issueNumber, 12); + } finally { + cleanup(rootDir); + } + }) + ) + passed++; + else failed++; + + if ( + await test('sync recovers coordinated issues whose epic label drifted', async () => { + const rootDir = createTempDir('github-coordination-sync-drift-'); + const dbPath = path.join(rootDir, 'state.db'); + + try { + const driftedIssue = { + number: 13, + title: 'Recover label drift', + body: '\n```json\n{}\n```\n', + url: 'https://github.com/affaan-m/ECC/issues/13', + state: 'OPEN', + labels: [{ name: 'coordination:synced' }], + author: { login: 'maintainer' }, + updatedAt: '2026-06-01T12:00:00Z' + }; + const shim = writeGhShim(rootDir, { + 'issue list --repo affaan-m/ECC --state all --limit 100 --label epic --json number,title,body,url,state,labels,author,updatedAt,assignees': [], + 'issue list --repo affaan-m/ECC --state all --limit 100 --search in:body "ecc-coordination:start" --json number,title,body,url,state,labels,author,updatedAt,assignees': [driftedIssue] + }); + + const result = run(['sync', '--repo', 'affaan-m/ECC', '--db', dbPath, '--dry-run', '--json'], { + cwd: rootDir, + env: { + ECC_GH_SHIM: shim.shimPath, + ECC_GH_SHIM_LOG: shim.logPath + } + }); + assert.strictEqual(result.status, 0, result.stderr); + const payload = parseJson(result.stdout); + assert.strictEqual(payload.count, 1); + assert.strictEqual(payload.items[0].issueNumber, 13); + assert.ok(payload.items[0].labels.includes('epic')); + } finally { + cleanup(rootDir); + } + }) + ) + passed++; + else failed++; + process.stdout.write(`\nResults: Passed: ${passed}, Failed: ${failed}\n`); process.exit(failed > 0 ? 1 : 0); } From 2083c9839a39c2ad0dee368d987126feb03b5650 Mon Sep 17 00:00:00 2001 From: Dante Date: Sat, 12 Sep 2026 08:46:38 +0800 Subject: [PATCH 05/72] fix(hooks): support Windows linter paths and ESLint 9 (#3076) pre-bash-commit-quality spawned Windows .cmd/.bat linters unquoted, so a spaced path failed, and passed --format compact, which ESLint 9 removed (#3075). Batch executables now run through cmd.exe with each argument carried in an env token and quoted, with quote, NUL, CR and LF rejected before spawn; non-batch Windows and POSIX paths keep direct argv spawn with shell false. ESLint uses its bundled default formatter, present on 8, 9 and 10. Regression tests cover the batch, non-batch and POSIX branches and the formatter change. Independent exact-head review passed with no P0/P1; CI 44/44 at the head. --- scripts/hooks/pre-bash-commit-quality.js | 92 +++++++++++-- tests/hooks/pre-bash-commit-quality.test.js | 135 +++++++++++++++++++- 2 files changed, 216 insertions(+), 11 deletions(-) diff --git a/scripts/hooks/pre-bash-commit-quality.js b/scripts/hooks/pre-bash-commit-quality.js index 5780c1d5b..400497055 100644 --- a/scripts/hooks/pre-bash-commit-quality.js +++ b/scripts/hooks/pre-bash-commit-quality.js @@ -259,20 +259,83 @@ function resolveCommand(command) { return null; } +const LINTER_TIMEOUT_MS = 30000; +const UNSAFE_CMD_TOKEN = /["\0\r\n]/; +const CMD_TOKEN_ENV_PREFIX = 'ECC_LINTER_TOKEN_'; + +function validateCmdToken(value) { + const token = String(value); + if (UNSAFE_CMD_TOKEN.test(token)) { + throw new Error(`Unsafe character in Windows linter argument: ${JSON.stringify(token)}`); + } + return token; +} + +function getLinterInvocation(command, args, platform = process.platform) { + const useCmd = platform === 'win32' && /\.(?:cmd|bat)$/i.test(command); + + if (useCmd) { + const environment = { ...process.env }; + for (const name of Object.keys(environment)) { + if (name.toUpperCase().startsWith(CMD_TOKEN_ENV_PREFIX)) { + delete environment[name]; + } + } + + // Keep untrusted values out of cmd.exe source. Percent expansion is + // non-recursive, so percent signs introduced by these environment values + // stay literal. Disabling delayed expansion likewise preserves exclamation + // marks. Quotes and line controls remain invalid because they could escape + // the quoted token boundary or create another command line. + const tokenReferences = [command, ...args].map((value, index) => { + const name = `${CMD_TOKEN_ENV_PREFIX}${index}`; + environment[name] = validateCmdToken(value); + return `"%${name}%"`; + }); + const commandLine = tokenReferences.join(' '); + return { + command: process.env.ComSpec || process.env.COMSPEC || 'cmd.exe', + args: ['/d', '/v:off', '/s', '/c', `"${commandLine}"`], + options: { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + timeout: LINTER_TIMEOUT_MS, + shell: false, + windowsVerbatimArguments: true, + env: environment + } + }; + } + + return { + command, + args, + options: { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + timeout: LINTER_TIMEOUT_MS, + shell: false + } + }; +} + function runLinterCommand(command, args) { - const useShell = process.platform === 'win32' && /\.(?:cmd|bat)$/i.test(command); - return spawnSync(command, args, { - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'pipe'], - timeout: 30000, - shell: useShell - }); + try { + const invocation = getLinterInvocation(command, args); + return spawnSync(invocation.command, invocation.args, invocation.options); + } catch (error) { + return { status: null, stdout: '', stderr: '', error }; + } } function commandOutput(result) { return result.stdout || result.stderr || result.error?.message || ''; } +function golintSucceeded(result) { + return result.status === 0 && !result.error && (!result.stdout || result.stdout.trim() === ''); +} + /** * Run linter on staged files * @param {string[]} files @@ -294,7 +357,7 @@ function runLinter(files) { const eslintBin = process.platform === 'win32' ? 'eslint.cmd' : 'eslint'; const eslintPath = path.join(process.cwd(), 'node_modules', '.bin', eslintBin); if (fs.existsSync(eslintPath)) { - const result = runLinterCommand(eslintPath, ['--format', 'compact', ...jsFiles]); + const result = runLinterCommand(eslintPath, jsFiles); results.eslint = { success: result.status === 0, output: commandOutput(result) @@ -329,7 +392,7 @@ function runLinter(files) { } else { const result = runLinterCommand(golintPath, goFiles); results.golint = { - success: !result.stdout || result.stdout.trim() === '', + success: golintSucceeded(result), output: commandOutput(result) }; } @@ -481,4 +544,13 @@ if (require.main === module) { }); } -module.exports = { run, evaluate, validateCommitMessage, findFileIssues, isPlaceholderSecret }; +module.exports = { + run, + evaluate, + validateCommitMessage, + findFileIssues, + isPlaceholderSecret, + getLinterInvocation, + golintSucceeded, + runLinter +}; diff --git a/tests/hooks/pre-bash-commit-quality.test.js b/tests/hooks/pre-bash-commit-quality.test.js index ebf17d26e..778fc2990 100644 --- a/tests/hooks/pre-bash-commit-quality.test.js +++ b/tests/hooks/pre-bash-commit-quality.test.js @@ -101,6 +101,7 @@ function withEnv(overrides, fn) { let passed = 0; let failed = 0; +let skipped = 0; console.log('\nPre-Bash Commit Quality Hook Tests'); console.log('==================================\n'); @@ -269,6 +270,138 @@ if (test('does not flag ordinary unquoted apiKey code references', () => { }); })) passed++; else failed++; +if (test('runs Windows batch linters through cmd with quoted command and arguments', () => { + const command = 'C:\\Users\\Jane %team%!\\project\\node_modules\\.bin\\eslint.cmd'; + const args = [ + 'index.js', + '100%.js', + '!important!.js', + '%PATH%.js', + '!PATH!.js', + '%1.js', + 'mixed %!^&() name.js' + ]; + const invocation = hook.getLinterInvocation(command, args, 'win32'); + + assert.ok(/cmd\.exe$/i.test(invocation.command)); + assert.deepStrictEqual(invocation.args, [ + '/d', + '/v:off', + '/s', + '/c', + '""%ECC_LINTER_TOKEN_0%" "%ECC_LINTER_TOKEN_1%" "%ECC_LINTER_TOKEN_2%" "%ECC_LINTER_TOKEN_3%" "%ECC_LINTER_TOKEN_4%" "%ECC_LINTER_TOKEN_5%" "%ECC_LINTER_TOKEN_6%" "%ECC_LINTER_TOKEN_7%""' + ]); + assert.deepStrictEqual( + Object.fromEntries(Object.entries(invocation.options.env).filter(([key]) => key.startsWith('ECC_LINTER_TOKEN_'))), + Object.fromEntries([command, ...args].map((value, index) => [`ECC_LINTER_TOKEN_${index}`, value])) + ); + assert.ok(!invocation.args[4].includes(command), 'untrusted command must not be embedded in cmd source'); + assert.ok(!invocation.args[4].includes(args[1]), 'untrusted argument must not be embedded in cmd source'); + assert.strictEqual(invocation.options.shell, false); + assert.strictEqual(invocation.options.windowsVerbatimArguments, true); + + const plainCmd = hook.getLinterInvocation('C:\\tools\\eslint.cmd', [], 'win32'); + assert.ok(/cmd\.exe$/i.test(plainCmd.command)); + assert.deepStrictEqual(plainCmd.args, ['/d', '/v:off', '/s', '/c', '""%ECC_LINTER_TOKEN_0%""']); + assert.strictEqual(plainCmd.options.shell, false); + + const batch = hook.getLinterInvocation('C:\\tools\\lint.BAT', [], 'win32'); + assert.ok(/cmd\.exe$/i.test(batch.command)); + assert.strictEqual(batch.options.shell, false); + + const executable = hook.getLinterInvocation('C:\\Program Files\\eslint.exe', [], 'win32'); + assert.strictEqual(executable.command, 'C:\\Program Files\\eslint.exe'); + assert.strictEqual(executable.options.shell, false); + + const posix = hook.getLinterInvocation('/tmp/project with spaces/eslint', [], 'darwin'); + assert.strictEqual(posix.command, '/tmp/project with spaces/eslint'); + assert.strictEqual(posix.options.shell, false); +})) passed++; else failed++; + +if (test('isolates Windows cmd token variables without mutating the parent environment', () => { + const original = process.env.ECC_LINTER_TOKEN_0; + process.env.ECC_LINTER_TOKEN_0 = 'parent value'; + + try { + const invocation = hook.getLinterInvocation('C:\\tools\\eslint.cmd', ['100%.js'], 'win32'); + assert.strictEqual(invocation.options.env.ECC_LINTER_TOKEN_0, 'C:\\tools\\eslint.cmd'); + assert.strictEqual(invocation.options.env.ECC_LINTER_TOKEN_1, '100%.js'); + assert.strictEqual(process.env.ECC_LINTER_TOKEN_0, 'parent value'); + } finally { + if (original === undefined) delete process.env.ECC_LINTER_TOKEN_0; + else process.env.ECC_LINTER_TOKEN_0 = original; + } +})) passed++; else failed++; + +if (process.platform === 'win32') { + if (test('passes percent and exclamation filenames literally to a Windows batch linter', () => { + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc cmd literal ')); + try { + const command = path.join(repoDir, 'lint %!.cmd'); + const capturePath = path.join(repoDir, 'captured arguments.txt'); + fs.writeFileSync(command, [ + '@echo off', + 'setlocal DisableDelayedExpansion', + '> "%ECC_CAPTURE_PATH%" echo(%~1', + '>> "%ECC_CAPTURE_PATH%" echo(%~2', + '' + ].join('\r\n'), 'utf8'); + + const invocation = hook.getLinterInvocation(command, ['100% ready.js', '!important!.js'], 'win32'); + const result = spawnSync(invocation.command, invocation.args, { + ...invocation.options, + env: { ...invocation.options.env, ECC_CAPTURE_PATH: capturePath } + }); + + assert.strictEqual(result.status, 0, result.stderr || result.error?.message); + assert.deepStrictEqual( + fs.readFileSync(capturePath, 'utf8').split(/\r?\n/).filter(Boolean), + ['100% ready.js', '!important!.js'] + ); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + })) passed++; else failed++; +} else { + console.log(' - passes percent and exclamation filenames literally to a Windows batch linter (skipped: Windows only)'); + skipped++; +} + +if (test('rejects characters that can break Windows cmd token boundaries', () => { + assert.throws( + () => hook.getLinterInvocation('C:\\tools\\eslint.cmd', ['bad"name.js'], 'win32'), + /Unsafe character/ + ); + assert.throws( + () => hook.getLinterInvocation('C:\\tools\\eslint.cmd', ['bad\r\nname.js'], 'win32'), + /Unsafe character/ + ); +})) passed++; else failed++; + +if (test('treats rejected or failed golint invocations as failures', () => { + assert.strictEqual(hook.golintSucceeded({ status: 0, stdout: '', error: null }), true); + assert.strictEqual(hook.golintSucceeded({ status: 0, stdout: 'issue.go:1: warning', error: null }), false); + assert.strictEqual(hook.golintSucceeded({ status: null, stdout: '', error: new Error('unsafe argument') }), false); +})) passed++; else failed++; + +if (test('uses ESLint bundled formatter without the removed compact formatter', () => { + inTempRepo(repoDir => { + const eslintPath = path.join(repoDir, 'node_modules', '.bin', executableName('eslint')); + fs.mkdirSync(path.dirname(eslintPath), { recursive: true }); + const source = process.platform === 'win32' + ? '@echo off\r\necho %* | findstr /C:"--format compact" >nul && exit /b 9\r\nexit /b 0\r\n' + : '#!/bin/sh\ncase " $* " in *" --format compact "*) exit 9 ;; esac\nexit 0\n'; + fs.writeFileSync(eslintPath, source, 'utf8'); + fs.chmodSync(eslintPath, 0o755); + + process.chdir(repoDir); + const result = hook.runLinter(['index.js']); + + assert.ok(result.eslint, 'expected ESLint to run'); + assert.strictEqual(result.eslint.success, true, result.eslint.output); + }); +})) passed++; else failed++; + if (test('reports eslint pylint and golint failures from staged files', () => { inTempRepo(repoDir => { writeAndStage(repoDir, 'index.js', 'const lint = true;\n'); @@ -372,5 +505,5 @@ if (test('measures length of the full message past an apostrophe (not the trunca assert.ok(res.issues.some(i => i.type === 'length'), 'full (>72) message should trigger a length issue'); })) passed++; else failed++; -console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); +console.log(`\nResults: Passed: ${passed}, Failed: ${failed}, Skipped: ${skipped}`); process.exit(failed > 0 ? 1 : 0); From 95b9fe157f815ef064793e49eacfecdbbfc5b813 Mon Sep 17 00:00:00 2001 From: Rockwell Windsor Rice Date: Fri, 11 Sep 2026 19:48:16 -0500 Subject: [PATCH 06/72] feat(rails-patterns): add Rails framework patterns skill (#3074) Adds skills/rails-patterns alongside laravel-patterns and django-patterns: directory contract, skinny controllers with service objects, form and query objects, idiomatic ActiveRecord, background jobs, ViewComponent, Hotwire, and the Rails 8 Solid stack. Decisions defer to rules/ruby/patterns.md. Registered in install-modules (framework-language), agent.yaml, package files, and every skill count (292) across plugin, marketplace, AGENTS and README variants. Independent exact-head review passed with no P0/P1; validate-skills, catalog:check, install manifests, plugin manifest, unicode and personal-path checks all pass; CI 44/44 at the head. --- .claude-plugin/marketplace.json | 2 +- .claude-plugin/plugin.json | 2 +- AGENTS.md | 4 +- README.md | 7 +- README.zh-CN.md | 2 +- agent.yaml | 1 + docs/tr/AGENTS.md | 4 +- docs/zh-CN/AGENTS.md | 4 +- docs/zh-CN/README.md | 6 +- manifests/install-modules.json | 1 + package.json | 1 + skills/rails-patterns/SKILL.md | 476 ++++++++++++++++++++++++++++++++ 12 files changed, 495 insertions(+), 15 deletions(-) create mode 100644 skills/rails-patterns/SKILL.md diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 3d2ff3e57..03b3f9f85 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -11,7 +11,7 @@ { "name": "ecc", "source": "./", - "description": "Harness-native ECC operator layer - 68 agents, 291 skills, 94 legacy command shims, reusable hooks, rules, selective install profiles, and production-ready workflows for Claude Code, Codex, OpenCode, Cursor, and related agent harnesses", + "description": "Harness-native ECC operator layer - 68 agents, 292 skills, 94 legacy command shims, reusable hooks, rules, selective install profiles, and production-ready workflows for Claude Code, Codex, OpenCode, Cursor, and related agent harnesses", "version": "2.2.1", "author": { "name": "Affaan Mustafa", diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 50a41a6f1..5f1e9a391 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "ecc", "version": "2.2.1", - "description": "Harness-native ECC plugin for engineering teams - 68 agents, 291 skills, 94 legacy command shims, reusable hooks, rules, MCP conventions, and operator workflows for Claude Code plus adjacent agent harnesses", + "description": "Harness-native ECC plugin for engineering teams - 68 agents, 292 skills, 94 legacy command shims, reusable hooks, rules, MCP conventions, and operator workflows for Claude Code plus adjacent agent harnesses", "author": { "name": "Affaan Mustafa", "url": "https://x.com/affaanmustafa" diff --git a/AGENTS.md b/AGENTS.md index 33605f894..085342923 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,6 @@ # Everything Claude Code (ECC) — Agent Instructions -This is a **production-ready AI coding plugin** providing 68 specialized agents, 291 skills, 94 commands, and automated hook workflows for software development. +This is a **production-ready AI coding plugin** providing 68 specialized agents, 292 skills, 94 commands, and automated hook workflows for software development. **Version:** 2.2.1 @@ -154,7 +154,7 @@ Troubleshoot failures: check test isolation → verify mocks → fix implementat ``` agents/ — 68 specialized subagents -skills/ — 291 workflow skills and domain knowledge +skills/ — 292 workflow skills and domain knowledge commands/ — 94 slash commands hooks/ — Trigger-based automations rules/ — Always-follow guidelines (common + per-language) diff --git a/README.md b/README.md index 9e4126dd3..e11cc084a 100644 --- a/README.md +++ b/README.md @@ -136,12 +136,12 @@ Instead of rebuilding that process in every prompt, you install it once and make ECC is MIT-licensed open source. It works best with Claude Code today, has a supported Codex sync path, and provides capability-limited adapters for Cursor, OpenCode, Gemini, Zed, GitHub Copilot, Antigravity, Qwen, and other harnesses. See the [support status matrix](#platform-support) before assuming feature parity. -Access to 68 agents, 291 skills, and 94 legacy command shims, plus hooks, rules, memory, continuous learning, and AgentShield security scanning. The agents are specialized for planning, review, build repair, security, architecture, and domain work. +Access to 68 agents, 292 skills, and 94 legacy command shims, plus hooks, rules, memory, continuous learning, and AgentShield security scanning. The agents are specialized for planning, review, build repair, security, architecture, and domain work. | Included | Count | What it gives you | | ---------------- | ----------: | ------------------------------------------------------------------------------------ | | Agents | 68 agents | Planning, review, build repair, security, architecture, and domain work | -| Skills | 291 skills | TDD, research, security, docs, frontend, data, ML, operations, and more | +| Skills | 292 skills | TDD, research, security, docs, frontend, data, ML, operations, and more | | Commands | 94 commands | Convenient entry points while ECC moves to a skills-first surface | | Hooks and memory | Runtime | Enforcement, session summaries, continuous learning, instincts, and context controls | | Rules | Selective | Always-loaded standards you choose by language or project | @@ -794,7 +794,7 @@ Stable graduation of the 2.0 line: control-pane substrate, worktree lifecycle se ```text ECC/ |-- agents/ # 68 specialized subagents for delegation -|-- skills/ # 291 reusable workflows loaded on demand +|-- skills/ # 292 reusable workflows loaded on demand |-- commands/ # 94 maintained slash-command shims |-- rules/ # opt-in common and language standards |-- hooks/ # runtime automation and enforcement @@ -887,6 +887,7 @@ ECC/ | |-- quarkus-security/ # Quarkus security | |-- quarkus-tdd/ # Quarkus TDD | |-- quarkus-verification/ # Quarkus verification +| |-- rails-patterns/ # Rails architecture patterns | |-- springboot-patterns/ # Java Spring Boot patterns | |-- springboot-security/ # Spring Boot security | |-- springboot-tdd/ # Spring Boot TDD diff --git a/README.zh-CN.md b/README.zh-CN.md index 214b978f7..e01fd54e2 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -196,7 +196,7 @@ Copy-Item -Recurse rules/typescript "$HOME/.claude/rules/" /plugin list ecc@ecc ``` -**完成！** 你现在可以使用 68 个代理、291 个技能和 94 个命令。 +**完成！** 你现在可以使用 68 个代理、292 个技能和 94 个命令。 ### multi-* 命令需要额外配置 diff --git a/agent.yaml b/agent.yaml index 3a1a48a59..ac7578d8d 100644 --- a/agent.yaml +++ b/agent.yaml @@ -125,6 +125,7 @@ skills: - quarkus-security - quarkus-tdd - quarkus-verification + - rails-patterns - ralphinho-rfc-pipeline - react-patterns - react-performance diff --git a/docs/tr/AGENTS.md b/docs/tr/AGENTS.md index 791e7f98a..c49b9962a 100644 --- a/docs/tr/AGENTS.md +++ b/docs/tr/AGENTS.md @@ -1,6 +1,6 @@ # Everything Claude Code (ECC) — Agent Talimatları -Bu, yazılım geliştirme için 68 özel agent, 291 skill, 94 command ve otomatik hook iş akışları sağlayan **üretime hazır bir AI kodlama eklentisidir**. +Bu, yazılım geliştirme için 68 özel agent, 292 skill, 94 command ve otomatik hook iş akışları sağlayan **üretime hazır bir AI kodlama eklentisidir**. **Sürüm:** 2.2.1 @@ -142,7 +142,7 @@ Başarısızlık sorunlarını giderin: test izolasyonunu kontrol edin → mockl ``` agents/ — 68 özel subagent -skills/ — 291 iş akışı skillleri ve alan bilgisi +skills/ — 292 iş akışı skillleri ve alan bilgisi commands/ — 94 slash command hooks/ — Tetikleyici tabanlı otomasyonlar rules/ — Her zaman uyulması gereken kurallar (ortak + dile özel) diff --git a/docs/zh-CN/AGENTS.md b/docs/zh-CN/AGENTS.md index e9141f30b..2f5a18856 100644 --- a/docs/zh-CN/AGENTS.md +++ b/docs/zh-CN/AGENTS.md @@ -1,6 +1,6 @@ # Everything Claude Code (ECC) — 智能体指令 -这是一个**生产就绪的 AI 编码插件**，提供 68 个专业代理、291 项技能、94 条命令以及自动化钩子工作流，用于软件开发。 +这是一个**生产就绪的 AI 编码插件**，提供 68 个专业代理、292 项技能、94 条命令以及自动化钩子工作流，用于软件开发。 **版本:** 2.2.1 @@ -147,7 +147,7 @@ ``` agents/ — 68 个专业子代理 -skills/ — 291 个工作流技能和领域知识 +skills/ — 292 个工作流技能和领域知识 commands/ — 94 个斜杠命令 hooks/ — 基于触发的自动化 rules/ — 始终遵循的指导方针（通用 + 每种语言） diff --git a/docs/zh-CN/README.md b/docs/zh-CN/README.md index 691c31a23..422f22d5d 100644 --- a/docs/zh-CN/README.md +++ b/docs/zh-CN/README.md @@ -260,7 +260,7 @@ Copy-Item -Recurse rules/typescript "$HOME/.claude/rules/" /plugin list ecc@ecc ``` -**搞定！** 你现在可以使用 68 个智能体、291 项技能和 94 个命令了。 +**搞定！** 你现在可以使用 68 个智能体、292 项技能和 94 个命令了。 *** @@ -1174,7 +1174,7 @@ opencode |---------|---------------|----------|--------| | 智能体 | PASS: 68 个 | PASS: 12 个 | **Claude Code 领先** | | 命令 | PASS: 94 个 | PASS: 35 个 | **Claude Code 领先** | -| 技能 | PASS: 291 项 | PASS: 37 项 | **Claude Code 领先** | +| 技能 | PASS: 292 项 | PASS: 37 项 | **Claude Code 领先** | | 钩子 | PASS: 8 种事件类型 | PASS: 11 种事件 | **OpenCode 更多！** | | 规则 | PASS: 29 条 | PASS: 13 条指令 | **Claude Code 领先** | | MCP 服务器 | PASS: 14 个 | PASS: 完整 | **完全对等** | @@ -1282,7 +1282,7 @@ ECC 是**第一个最大化利用每个主要 AI 编码工具的插件**。以 |---------|-----------------------|------------|-----------|----------| | **智能体** | 68 | 共享 (AGENTS.md) | 共享 (AGENTS.md) | 12 | | **命令** | 94 | 共享 | 基于指令 | 35 | -| **技能** | 291 | 共享 | 10 (原生格式) | 37 | +| **技能** | 292 | 共享 | 10 (原生格式) | 37 | | **钩子事件** | 8 种类型 | 15 种类型 | SessionStart（1 种类型） | 11 种类型 | | **钩子脚本** | 20+ 个脚本 | 16 个脚本 (DRY 适配器) | 1 个 SessionStart 引导脚本 | 插件钩子 | | **规则** | 34 (通用 + 语言) | 34 (YAML 前页) | 基于指令 | 13 条指令 | diff --git a/manifests/install-modules.json b/manifests/install-modules.json index 92a73e08d..884c7d39d 100644 --- a/manifests/install-modules.json +++ b/manifests/install-modules.json @@ -196,6 +196,7 @@ "skills/quarkus-patterns", "skills/quarkus-tdd", "skills/quarkus-verification", + "skills/rails-patterns", "skills/react-patterns", "skills/react-performance", "skills/react-testing", diff --git a/package.json b/package.json index b3508b59a..87487a914 100644 --- a/package.json +++ b/package.json @@ -302,6 +302,7 @@ "skills/quarkus-security/", "skills/quarkus-tdd/", "skills/quarkus-verification/", + "skills/rails-patterns/", "skills/ralphinho-rfc-pipeline/", "skills/react-patterns/", "skills/react-performance/", diff --git a/skills/rails-patterns/SKILL.md b/skills/rails-patterns/SKILL.md new file mode 100644 index 000000000..5133e8ab4 --- /dev/null +++ b/skills/rails-patterns/SKILL.md @@ -0,0 +1,476 @@ +--- +name: rails-patterns +description: Ruby on Rails framework patterns for Rails 7.1+ and 8.x apps. Covers the directory contract, skinny controllers with service objects, form objects, query objects, idiomatic ActiveRecord, background jobs, ViewComponent, Hotwire, and the Rails 8 Solid stack. Use when building or reviewing Rails apps, controllers, models, services, jobs, or views. +origin: community +--- + +# Rails Patterns + +Framework patterns for modern Ruby on Rails applications (Rails 7.1+ and 8.x). Rails is opinionated by design; these are the patterns the community has converged on for apps that stay maintainable past the 50-model mark. This skill is the "how." For the "what" and "when" (the decisions about which pattern to reach for), see the Ruby patterns rules — `rules/ruby/patterns.md` in this repository, installed as `rules/ecc/ruby/patterns.md`. + +## When to Activate + +- Building a Rails application (full-stack, API-only, or hybrid) +- Reviewing a PR that touches `app/` or `config/` +- Generating models, controllers, services, or jobs +- A controller action grows past ~10 lines +- A model file grows past ~200 lines +- ActiveRecord queries start appearing in controllers or views + +## Core Concepts + +### The directory contract + +Rails apps follow a predictable structure. Add directories deliberately, not casually. + +``` +app/ + models/ ActiveRecord models. Persistence and domain logic close to the data. + controllers/ HTTP request handling. Thin orchestration only. + views/ ERB templates. No business logic. + components/ ViewComponent classes. View logic that needs tests. + services/ Service objects. Multi-step business operations. + forms/ Form objects. Complex form handling across multiple models. + queries/ Query objects. Reusable, composable ActiveRecord queries. + jobs/ Background jobs. Async work via Solid Queue, Sidekiq, or GoodJob. + mailers/ ActionMailer classes. + helpers/ View helpers. Tiny presentational logic only. + policies/ Authorization policies (if using Pundit). Optional. + channels/ ActionCable channels for WebSocket work. +``` + +Avoid `app/lib/`, `app/utils/`, `app/managers/`. If something does not fit the directories above, the design usually needs rethinking, not a new directory. Truly generic code goes in `lib/`. + +### Skinny controllers + +Controllers receive a request, delegate to the right object, and render a response. Business logic lives elsewhere. (Per the Ruby patterns rules, extract to a service object when the controller starts carrying multiple responsibilities.) + +### Service objects + +The default for business operations that touch more than a single model save. Conventions that keep them consistent: + +- Namespace by domain (`Invoices::Create`), not by suffix (`InvoiceCreator`). +- A class method `.call` delegates to an instance `#call`. +- Return a Result object, not a boolean or a bare record, so the caller can branch on success, errors, and the affected record. +- Wrap multi-record writes in a transaction. +- Keep each service single-purpose (`Invoices::Create`, `Invoices::MarkPaid`), never `Invoices::Manager`. + +### Form objects + +When a form spans multiple models or has fields that do not map to columns, use a form object rather than nested attributes or virtual attributes on the wrong model. It quacks like a model to the view (`form_with model: @form`) while composing records cleanly. + +### Query objects + +For ActiveRecord queries reused across controllers or services, or too complex for a scope, extract a query object that accepts a scope as input so it composes. Rule of thumb: a scope that grows past three chained conditions or starts taking parameters wants to be a query object. + +### Background jobs + +Offload anything slow. (Per the Ruby patterns rules, Solid Queue for greenfield Rails 8 with modest throughput; Sidekiq when you need mature observability, high throughput, or existing Redis.) Regardless of adapter: pass IDs not records, make `perform` idempotent, and set `retry_on`/`discard_on` explicitly. + +### ViewComponent over partials + +For view logic with conditional rendering, more than two arguments, or reuse across more than three places, prefer a ViewComponent. Components are testable in isolation and surface their interface explicitly; partials with deep conditional logic become debt. + +### Hotwire: Turbo and Stimulus + +The default Rails frontend stack. (Per the Ruby patterns rules, prefer Hotwire for server-rendered apps; reach for React/Vue only when interaction complexity justifies the client surface.) Turbo Frames for partial page updates, Turbo Streams for server-driven updates, Stimulus for small client-side behaviors next to the markup. + +### The Rails 8 Solid stack + +Rails 8 ships database-backed defaults that previously needed Redis: Solid Queue (jobs), Solid Cache (cache), Solid Cable (ActionCable). The tradeoff is more database load for one fewer infrastructure component; a good fit for modest throughput, with Redis still winning at high scale. Kamal is the default Docker-based deploy tool. + +## Code Examples + +### Skinny controller with a service object + +```ruby +# Bad: business logic in the controller +class InvoicesController < ApplicationController + def create + @invoice = Invoice.new(invoice_params) + @invoice.user = current_user + @invoice.line_items.build(invoice_params[:line_items]) + @invoice.tax_total = TaxCalculator.new(@invoice).calculate + @invoice.total = @invoice.line_items.sum(&:amount) + @invoice.tax_total + + if @invoice.save + InvoiceMailer.created(@invoice).deliver_later + AccountingExportJob.perform_later(@invoice.id) + redirect_to @invoice, notice: "Invoice created" + else + render :new + end + end +end + +# Good: controller orchestrates, service does the work +class InvoicesController < ApplicationController + def create + result = Invoices::Create.call(params: invoice_params, user: current_user) + + if result.success? + redirect_to result.invoice, notice: "Invoice created" + else + @invoice = result.invoice + render :new, status: :unprocessable_entity + end + end +end +``` + +### The service object + +```ruby +# app/services/invoices/create.rb +module Invoices + class Create + # Struct keeps this runnable on every Ruby that Rails 7.1 supports. + # On Ruby 3.2+, `Data.define(:success?, :invoice, :errors)` is a more + # concise immutable alternative. + Result = Struct.new(:success, :invoice, :errors, keyword_init: true) do + def success? + success + end + end + + def self.call(params:, user:) + new(params: params, user: user).call + end + + def initialize(params:, user:) + @params = params + @user = user + end + + def call + invoice = build_invoice + ApplicationRecord.transaction do + invoice.save! + end + begin + send_notifications(invoice) + rescue StandardError => e + Rails.logger.error("Notification dispatch failed for invoice #{invoice.id}: #{e.message}") + end + Result.new(success: true, invoice: invoice, errors: nil) + rescue ActiveRecord::RecordInvalid => e + Result.new(success: false, invoice: e.record, errors: e.record.errors) + end + + private + + attr_reader :params, :user + + def build_invoice + invoice = user.invoices.new(params.except(:line_items)) + invoice.tax_total = TaxCalculator.call(invoice) + invoice.line_items.build(params[:line_items]) + invoice.total = invoice.line_items.sum(&:amount) + invoice.tax_total + invoice + end + + def send_notifications(invoice) + InvoiceMailer.created(invoice).deliver_later + AccountingExportJob.perform_later(invoice.id) + end + end +end +``` + +### Form object + +```ruby +# app/forms/signup_form.rb +class SignupForm + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :email, :string + attribute :password, :string + attribute :company_name, :string + attribute :terms_accepted, :boolean + + validates :email, presence: true, format: URI::MailTo::EMAIL_REGEXP + validates :password, presence: true, length: { minimum: 12 } + validates :company_name, presence: true + validates :terms_accepted, acceptance: true + + attr_reader :user, :company + + def save + return false unless valid? + + ApplicationRecord.transaction do + @company = Company.create!(name: company_name) + @user = @company.users.create!(email: email, password: password, role: :owner) + end + true + rescue ActiveRecord::RecordInvalid => e + errors.merge!(e.record.errors) + false + end +end +``` + +### Query object + +```ruby +# app/queries/invoices/overdue.rb +module Invoices + class Overdue + def self.call(scope: Invoice.all, as_of: Time.current) + new(scope: scope, as_of: as_of).call + end + + def initialize(scope:, as_of:) + @scope = scope + @as_of = as_of + end + + def call + scope + .where(status: :sent) + .where(due_date: ..as_of) + .where.not(id: paid_invoice_ids) + .includes(:customer, :line_items) + end + + private + + attr_reader :scope, :as_of + + def paid_invoice_ids + Payment.where(created_at: ..as_of).pluck(:invoice_id) + end + end +end +``` + +Query objects accept a scope, so they compose: `Invoices::Overdue.call(scope: current_user.invoices)`. + +### N+1 prevention + +```ruby +# Bad: N+1 in the view when it calls post.author.name +@posts = Post.published + +# Good: eager load +@posts = Post.published.includes(:author) +``` + +`includes` lets Rails choose preload vs eager_load. Force `preload` for separate queries, `eager_load` for a JOIN when filtering on the association. In Rails 7.1+, `strict_loading` raises on accidental lazy loads. + +### Counter cache + +```ruby +class Comment < ApplicationRecord + belongs_to :post, counter_cache: true +end +``` + +```ruby +add_column :posts, :comments_count, :integer, default: 0, null: false +``` + +`post.comments_count` becomes a column read instead of a `COUNT(*)`. This example +assumes a new table; adding a counter cache to a table that already has rows requires a +backfill, which is out of scope here. + +### Background job shape + +Pass record IDs, not records. Retries make delivery at-least-once, so any job that calls +an external service must be idempotent — otherwise a transient failure after the remote +call succeeds will duplicate the effect on the next attempt. + +```ruby +class AccountingExportJob < ApplicationJob + queue_as :exports + + retry_on AccountingApi::TransientError, wait: :polynomially_longer, attempts: 5 + discard_on AccountingApi::PermanentError + + def perform(invoice_id) + invoice = Invoice.find(invoice_id) + export = AccountingExport.create_or_find_by!( + invoice: invoice, + idempotency_key: "invoice-export-#{invoice.id}-#{invoice.updated_at.to_i}" + ) + return if export.completed_at? + + receipt = AccountingApi.export(invoice, idempotency_key: export.idempotency_key) + export.update!(completed_at: Time.current, external_id: receipt.id) + end +end +``` + +```ruby +add_index :accounting_exports, :idempotency_key, unique: true +``` + +The unique index is what makes this safe: when two attempts race, the database rejects +the second insert and Active Record resolves the conflict inside the call, returning the +existing row. That happens without any job-level retry — `retry_on` above covers only +`AccountingApi::TransientError`. The guard +covers the window before the remote call; passing `idempotency_key` through to the API +covers the window after it, so a crash between the API call and `update!` still resolves +to a single export. + +### ViewComponent + +```ruby +# app/components/invoice_status_badge_component.rb +class InvoiceStatusBadgeComponent < ViewComponent::Base + STATUS_CLASSES = { + draft: "bg-gray-100 text-gray-800", + sent: "bg-blue-100 text-blue-800", + paid: "bg-green-100 text-green-800", + overdue: "bg-red-100 text-red-800" + }.freeze + + def initialize(invoice:) + @invoice = invoice + end + + def call + tag.span(@invoice.status.humanize, class: "rounded-full px-2 py-1 text-sm #{status_class}") + end + + private + + def status_class + STATUS_CLASSES.fetch(@invoice.status.to_sym, "bg-gray-100") + end +end +``` + +```erb +<%= render InvoiceStatusBadgeComponent.new(invoice: @invoice) %> +``` + +### Hotwire + +```erb +<%# Turbo Frame: clicking Edit replaces only this frame %> +<%= turbo_frame_tag "invoice_#{@invoice.id}" do %> +
+ <%= link_to "Edit", edit_invoice_path(@invoice) %> +
+<% end %> +``` + +```erb +<%# Turbo Stream: app/views/comments/create.turbo_stream.erb %> +<%= turbo_stream.append "comments", @comment %> +<%= turbo_stream.update "comment_form", partial: "form", locals: { comment: Comment.new } %> +``` + +```javascript +// app/javascript/controllers/copy_to_clipboard_controller.js +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = ["source"] + + copy() { + navigator.clipboard.writeText(this.sourceTarget.value) + } +} +``` + +### Acceptable vs unacceptable callbacks + +```ruby +# Acceptable: pure data normalization +class User < ApplicationRecord + before_validation :normalize_email + + private + + def normalize_email + self.email = email.to_s.downcase.strip + end +end + +# Move to a service instead: side effects hidden in a callback +# class User < ApplicationRecord +# after_create :send_welcome_email # hard to opt out of, hard to test +# end +``` + +### Good concern vs bad concern + +```ruby +# Good: genuinely cross-cutting, reusable across unrelated models +# app/models/concerns/soft_deletable.rb +module SoftDeletable + extend ActiveSupport::Concern + + included do + scope :active, -> { where(deleted_at: nil) } + scope :deleted, -> { where.not(deleted_at: nil) } + end + + def soft_delete! = update!(deleted_at: Time.current) + def restore! = update!(deleted_at: nil) +end + +# Bad: a "concern" used by exactly one model, holding logic that belongs on it +# app/models/concerns/invoice_calculations.rb +module InvoiceCalculations + extend ActiveSupport::Concern + + def calculate_total + line_items.sum(&:amount) + tax_total + end +end +# Only Invoice includes this. It isn't cross-cutting; it's Invoice's own logic +# hidden in a module for the appearance of a "skinny" model. Put it back on Invoice. +``` + +A concern used by only one class is just moving code; it belongs in that class. A concern should be reusable across at least two unrelated models. + +## Anti-Patterns + +### God controllers + +Any controller past ~80 lines is doing too much. Split actions across controllers or extract to services. + +### Fat models with 30+ methods + +Models should know about their own data. Methods that orchestrate other models, send notifications, or coordinate workflows belong in services. + +### Callback chains + +`after_save :update_cache, :send_notifications, :enqueue_export` is the start of a debugging nightmare. Move them into a service that runs them explicitly. + +### Nested attributes for complex forms + +`accepts_nested_attributes_for` is fine for simple cases. For conditional validation or cross-model logic, use a form object. + +### Default scopes on critical models + +`default_scope { where(deleted: false) }` silently excludes records from every query in the app, including the ones you need for support and debugging. Prefer an explicit named scope. + +### Models named after database concepts + +`UserRole`, `OrderStatus`, `InvoiceState` are usually enum candidates, not models. + +### Reaching for a JS framework before Hotwire + +If the page is server-rendered with occasional interactivity, Hotwire ships faster. Reserve React/Vue for genuinely SPA-shaped apps. + +## Best Practices + +- Keep controllers thin; push business logic into services. +- Return Result objects from services so callers branch on outcome, not exceptions. +- Wrap multi-record writes in a transaction; let notification/side-effect failures log without breaking the primary write. +- Pass IDs to jobs, keep `perform` idempotent, set retry/discard explicitly. +- Default to eager loading; treat an accidental N+1 as a bug, not a nuisance. +- Reserve concerns for behavior shared across at least two unrelated models. +- Reach for Hotwire before a client-side framework on server-rendered apps. + +## Related Skills + +- `backend-patterns` — service boundaries and adapter patterns (referenced by the Ruby patterns rules) +- `ruby-patterns` — language-level Ruby idioms (if present) +- Ruby patterns rules (`rules/ruby/patterns.md`, installed as `rules/ecc/ruby/patterns.md`) — the decisions and when-to-use guidance this skill implements From 4f373874209b4b63fdec0469b76992923ea0a9d0 Mon Sep 17 00:00:00 2001 From: Zaal Date: Fri, 11 Sep 2026 22:07:29 -0400 Subject: [PATCH 07/72] fix(hooks): block-no-verify handles stuck optional values and long-option prefixes (#3073) Two cases the word-level rewrite still got wrong. Short options that take an optional stuck value (-u[mode], -S[keyid]) end the cluster scan, so git commit -uno and -Sn are allowed while -nu stays blocked. Git accepts any unambiguous long-option prefix, so --no-veri and --no-verif on commit, push, merge and rebase are now blocked; --no-verbose stays allowed. Quoted data such as -m "--no-verify" is still treated as data. Independent exact-head review probed 34 commands in-process and against real git with no bypass and no false positive; hook test 35/35, eslint clean, CI 44/44 at the head. --- scripts/hooks/block-no-verify.js | 17 ++++++++++++++- tests/hooks/block-no-verify.test.js | 32 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js index ecd29100c..16e0044d7 100644 --- a/scripts/hooks/block-no-verify.js +++ b/scripts/hooks/block-no-verify.js @@ -78,6 +78,10 @@ const COMMIT_OPTIONS_WITH_INLINE_VALUE = [ // must stop at this character — anything after it is the inline value, // not another flag. const COMMIT_SHORT_OPTIONS_WITH_VALUE = new Set(['m', 'F', 'C', 'c', 't']); +// Short options whose value is OPTIONAL and must be stuck to the flag +// (`-uno`, `-S`). The rest of the cluster is that value, so an `n` +// after them is not the -n flag: `git commit -uno` means --untracked-files=no. +const COMMIT_SHORT_OPTIONS_WITH_OPTIONAL_VALUE = new Set(['u', 'S']); function tokenizeShellWords(input, start = 0, end = input.length) { const tokens = []; @@ -264,6 +268,7 @@ function isCommitNoVerifyShortFlag(value) { const option = options.charAt(i); if (option === 'n') return true; if (COMMIT_SHORT_OPTIONS_WITH_VALUE.has(option)) return false; + if (COMMIT_SHORT_OPTIONS_WITH_OPTIONAL_VALUE.has(option)) return false; } return false; @@ -388,6 +393,16 @@ function detectGitCommand(input, start = 0) { return null; } +/** + * git's option parser accepts any unambiguous prefix of a long option, so + * `--no-veri` and `--no-verif` run as --no-verify. Shorter prefixes such as + * `--no-ver` are ambiguous with --no-verbose and git rejects them itself, so + * refusing every prefix from `--no-v` up blocks nothing that would have run. + */ +function isNoVerifyLongFlag(value) { + return value.length >= '--no-v'.length && '--no-verify'.startsWith(value); +} + /** * Check if the input contains a --no-verify flag for a specific git command. * Only inspects the portion of the input starting at `offset` (the position @@ -422,7 +437,7 @@ function hasNoVerifyFlag(input, command, offset) { } } - if (value === '--no-verify') return true; + if (isNoVerifyLongFlag(value)) return true; // For commit, -n is shorthand for --no-verify. if (command === 'commit' && isCommitNoVerifyShortFlag(value)) { diff --git a/tests/hooks/block-no-verify.test.js b/tests/hooks/block-no-verify.test.js index db38dbb19..8b07d5f00 100644 --- a/tests/hooks/block-no-verify.test.js +++ b/tests/hooks/block-no-verify.test.js @@ -219,6 +219,38 @@ if (test('still allows -tn (n is the -t template path, not a flag)', () => { assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); })) passed++; else failed++; +// --- Optional stuck values (-u, -S) and long-option prefixes --- + +if (test('allows -uno (n is the -u untracked-files mode, not a flag)', () => { + const r = runHook({ tool_input: { command: 'git commit -uno -m "msg"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('allows -Sn (n is the -S key id, not a flag)', () => { + const r = runHook({ tool_input: { command: 'git commit -Sn -m "msg"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + +if (test('still blocks -nu (n comes before the optional-value flag)', () => { + const r = runHook({ tool_input: { command: 'git commit -nu -m "msg"' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); +})) passed++; else failed++; + +if (test('blocks --no-veri (git accepts unambiguous long-option prefixes)', () => { + const r = runHook({ tool_input: { command: 'git commit --no-veri -m "msg"' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); +})) passed++; else failed++; + +if (test('blocks --no-verif on git push', () => { + const r = runHook({ tool_input: { command: 'git push --no-verif origin main' } }); + assert.strictEqual(r.code, 2, `expected exit 2, got ${r.code}`); +})) passed++; else failed++; + +if (test('allows --no-verbose (not a prefix of --no-verify)', () => { + const r = runHook({ tool_input: { command: 'git commit --no-verbose -m "msg"' } }); + assert.strictEqual(r.code, 0, `expected exit 0, got ${r.code}: ${r.stderr}`); +})) passed++; else failed++; + console.log('─'.repeat(50)); console.log(`Passed: ${passed} Failed: ${failed}`); From bdf92d8fab701ac31990ada6250ac15757ab0451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Kaznowski?= Date: Sat, 12 Sep 2026 05:20:03 +0200 Subject: [PATCH 08/72] docs: add explicit hook consent to OpenCode install command (#3080) The README's OpenCode full-profile install command fails because assertHookConsentReady refuses to materialize hooks-runtime without explicit consent; --profile full selects the runtime so the gate fires. Add --enable-hooks to the documented command. Independent review reproduced the failure and the fix by running the installer in a scratch HOME; CI 44/44 at the head. docs/uk-UA/README.md still carries the old command and can follow. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e11cc084a..86dba5120 100644 --- a/README.md +++ b/README.md @@ -369,7 +369,7 @@ cd ECC | Harness | Install or setup | Notes | |---|---|---| | Cursor | `./install.sh --profile minimal --target cursor` | Project-local `.cursor/` adapter | -| OpenCode | `npm install && npm run build:opencode && ./install.sh --profile full --target opencode` | Builds the plugin payload before the full install | +| OpenCode | `npm install && npm run build:opencode && ./install.sh --profile full --target opencode --enable-hooks` | Builds the plugin payload before the full install | | Gemini CLI | `./install.sh --profile minimal --target gemini` | Project-local `.gemini/` config | | Zed | `./install.sh --profile minimal --target zed` | Project-local `.zed/` adapter | | Antigravity | `./install.sh --profile minimal --target antigravity` | See the [Antigravity guide](docs/ANTIGRAVITY-GUIDE.md) | From fba8e352cc8e2847b7b0f9289d868f399e8fb066 Mon Sep 17 00:00:00 2001 From: Serply <59339358+googio@users.noreply.github.com> Date: Fri, 11 Sep 2026 23:32:09 -0400 Subject: [PATCH 09/72] feat(mcp): add serply-search server catalog entry (#3085) Adds an opt-in HTTP entry for Serply's hosted MCP endpoint to mcp-configs/mcp-servers.json, following the parallel-search and browser-use shape: https URL, X-Api-Key header placeholder, nothing enabled by default. Independent review confirmed valid JSON, catalog-tier policy per docs/MCP-CONNECTOR-POLICY.md, and that the endpoint itself answers 401 asking for X-Api-Key, so the placeholder header is warranted. CI 44/44 at the head. --- mcp-configs/mcp-servers.json | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/mcp-configs/mcp-servers.json b/mcp-configs/mcp-servers.json index 0e8854680..724a0089c 100644 --- a/mcp-configs/mcp-servers.json +++ b/mcp-configs/mcp-servers.json @@ -213,6 +213,14 @@ "command": "npx", "args": ["-y", "squish-memory"], "description": "Local-first persistent memory runtime for AI agents — MCP server for Claude Code, Cursor, OpenCode, Codex, Cline. Auto-captures context across sessions. 1-20ms recall, 283KB, no second LLM needed. Runs locally with SQLite. Supports cloud sync via Stripe checkout ($9-$99/mo). GitHub: https://github.com/michielhdoteth/squish | Docs: https://squishplugin.dev | (also available via local `squish run mcp`)" + }, + "serply-search": { + "type": "http", + "url": "https://api.serply.io/mcp", + "headers": { + "X-Api-Key": "YOUR_SERPLY_API_KEY_HERE" + }, + "description": "Serply hosted web search: Google, Bing, News, Scholar, Jobs, Maps, video and Amazon product search plus scrape_url. Search tools return a readable result list with structured content alongside, Maps returns structured JSON, and scrape_url returns Markdown or raw HTML. Complements exa-web-search and parallel-search with classic keyword SERP results and per-country/locale targeting. Requires an API key from serply.io (replace the header placeholder)." } }, "_comments": { From c4904e3f6381df934fc00bffb0afa7a1f8dae0e3 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Sat, 12 Sep 2026 04:45:33 +0100 Subject: [PATCH 10/72] fix(catalog): remove Serply and Squish entries (#3094) Remove the two reference catalog entries and their promotional descriptions. The catalog retains its other 34 entries unchanged. Validated JSON structure, exact 13-line deletion, tracked references and independent code/security review. --- mcp-configs/mcp-servers.json | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/mcp-configs/mcp-servers.json b/mcp-configs/mcp-servers.json index 724a0089c..f3d607bd7 100644 --- a/mcp-configs/mcp-servers.json +++ b/mcp-configs/mcp-servers.json @@ -208,19 +208,6 @@ "OPENAI_API_KEY": "YOUR_OPENAI_API_KEY_HERE" }, "description": "AI agent regression testing — snapshot behavior, detect regressions in tool calls and output quality. 8 tools: create_test, run_snapshot, run_check, list_tests, validate_skill, generate_skill_tests, run_skill_test, generate_visual_report. API key optional — deterministic checks (tool diff, output hash) work without it. Install: pip install \"evalview>=0.5,<1\"" - }, - "squish": { - "command": "npx", - "args": ["-y", "squish-memory"], - "description": "Local-first persistent memory runtime for AI agents — MCP server for Claude Code, Cursor, OpenCode, Codex, Cline. Auto-captures context across sessions. 1-20ms recall, 283KB, no second LLM needed. Runs locally with SQLite. Supports cloud sync via Stripe checkout ($9-$99/mo). GitHub: https://github.com/michielhdoteth/squish | Docs: https://squishplugin.dev | (also available via local `squish run mcp`)" - }, - "serply-search": { - "type": "http", - "url": "https://api.serply.io/mcp", - "headers": { - "X-Api-Key": "YOUR_SERPLY_API_KEY_HERE" - }, - "description": "Serply hosted web search: Google, Bing, News, Scholar, Jobs, Maps, video and Amazon product search plus scrape_url. Search tools return a readable result list with structured content alongside, Maps returns structured JSON, and scrape_url returns Markdown or raw HTML. Complements exa-web-search and parallel-search with classic keyword SERP results and per-country/locale targeting. Requires an API key from serply.io (replace the header placeholder)." } }, "_comments": { From 1ac07903ec993f89757b59912d7fb31a366953bd Mon Sep 17 00:00:00 2001 From: zpearce-2814 Date: Fri, 11 Sep 2026 23:59:02 -0700 Subject: [PATCH 11/72] fix(hooks): keep hooks.json within Claude Code's schema Move stable hook metadata to a validated sidecar while preserving hook commands and installer identity. Reject moved fingerprints and duplicate IDs, and validate before updating metadata. Independent local review passed at c315271624a1fd055b992f2bff889ad2a0ff8a6b; CI run 34678210149 passed. Rollback: revert this squash commit. --- hooks/README.md | 4 + hooks/hooks.json | 97 ++---- hooks/hooks.metadata.json | 139 ++++++++ schemas/hooks-metadata.schema.json | 67 ++++ scripts/ci/validate-hooks.js | 169 +++++++++- scripts/dashboard-web.js | 5 +- scripts/lib/hooks-config.js | 318 ++++++++++++++++++ scripts/lib/install-lifecycle.js | 22 +- scripts/lib/install-targets/helpers.js | 5 +- scripts/lib/install/plan.js | 5 +- ...continuous-learning-observe-runner.test.js | 3 +- tests/hooks/hooks-metadata.test.js | 293 ++++++++++++++++ tests/hooks/hooks.test.js | 7 +- tests/hooks/posttooluse-dispatcher.test.js | 11 +- tests/hooks/skill-run-tracker.test.js | 5 +- tests/hooks/stop-hooks-stdout.test.js | 5 +- tests/integration/hooks.test.js | 3 +- tests/lib/install-lifecycle.test.js | 3 +- tests/plugin-manifest.test.js | 3 +- 19 files changed, 1060 insertions(+), 104 deletions(-) create mode 100644 hooks/hooks.metadata.json create mode 100644 schemas/hooks-metadata.schema.json create mode 100644 scripts/lib/hooks-config.js create mode 100644 tests/hooks/hooks-metadata.test.js diff --git a/hooks/README.md b/hooks/README.md index 144bc89ad..620ef981f 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -19,6 +19,10 @@ User request → Claude picks a tool → PreToolUse hook runs → Tool executes Memory persistence lifecycle definitions live in `hooks/memory-persistence/`. The executable hook graph remains `hooks/hooks.json`; the memory persistence directory is the stable contract for SessionStart, PreCompact, observation, activity tracking, and SessionEnd behavior. +Stable hook IDs and descriptions live in `hooks/hooks.metadata.json`, aligned by event and index with `hooks/hooks.json`. Claude Code validates a plugin's `hooks.json` against its own schema and reports any other key (`$schema`, `id`, `description`) as unknown at load time, so `hooks.json` carries only what the harness accepts. ECC's installer, validator, and dashboard merge the sidecar back in through `scripts/lib/hooks-config.js`; `node scripts/ci/validate-hooks.js` fails if the two files drift apart. + +Each sidecar entry also carries a `fingerprint` of the matcher entry it describes (matcher plus hook commands), so reordering `hooks.json` without reordering the sidecar, or editing a command without updating the sidecar, is caught rather than silently swapping IDs. When reordering hooks, move the matching sidecar entries first. Then run `node scripts/ci/validate-hooks.js --update-fingerprints` to refresh changed commands and commit both files. The updater rejects known fingerprints at different positions and writes only after validation succeeds. + ## Installing These Hooks Manually For Claude Code manual installs, do not paste the raw repo `hooks.json` into `~/.claude/settings.json` or copy it directly into `~/.claude/hooks/hooks.json`. The checked-in file is plugin/repo-oriented and is meant to be installed through the ECC installer or loaded as a plugin. diff --git a/hooks/hooks.json b/hooks/hooks.json index 62053904e..641873396 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -1,5 +1,4 @@ { - "$schema": "https://json.schemastore.org/claude-code-settings.json", "hooks": { "PreToolUse": [ { @@ -9,9 +8,7 @@ "type": "command", "command": "node -e \"const p=require('path');const r=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i{let pending=1;const done=()=>{pending-=1;if(pending===0)process.exit(code);};if(out){pending+=1;process.stdout.write(out,done);}if(err){pending+=1;process.stderr.write(err,done);}process.nextTick(done);};const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i{let pending=1;const done=()=>{pending-=1;if(pending===0)process.exit(code);};if(out){pending+=1;process.stdout.write(out,done);}if(err){pending+=1;process.stderr.write(err,done);}process.nextTick(done);};const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i{let pending=1;const done=()=>{pending-=1;if(pending===0)process.exit(code);};if(out){pending+=1;process.stdout.write(out,done);}if(err){pending+=1;process.stderr.write(err,done);}process.nextTick(done);};const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i { + if (!matcher || typeof matcher !== 'object') return; + for (const key of HARNESS_UNKNOWN_MATCHER_KEYS) { + if (key in matcher) { + console.error( + `ERROR: hooks.json ${eventType}[${index}] must not define "${key}" - ` + + `move it to ${METADATA_FILENAME}` + ); + hasErrors = true; + } + } + }); + } + + return hasErrors; +} + +/** + * Validate a parsed document against a JSON schema file, if the schema exists. + * + * @param {object} document - Parsed JSON to validate. + * @param {string} schemaPath - Path to the schema; skipped when absent. + * @param {string} label - Name used in error output. + * @returns {boolean} true if errors were found + */ +function validateAgainstSchema(document, schemaPath, label) { + if (!fs.existsSync(schemaPath)) { + return false; + } + const schema = JSON.parse(fs.readFileSync(schemaPath, 'utf-8')); + const ajv = new Ajv({ allErrors: true }); + const validate = ajv.compile(schema); + if (validate(document)) { + return false; + } + for (const err of validate.errors) { + console.error(`ERROR: ${label} schema: ${err.instancePath || '/'} ${err.message}`); + } + return true; +} + function validateHooks() { if (!fs.existsSync(HOOKS_FILE)) { console.log('No hooks.json found, skipping validation'); @@ -138,18 +251,51 @@ function validateHooks() { process.exit(1); } - // Validate against JSON schema - if (fs.existsSync(HOOKS_SCHEMA_PATH)) { - const schema = JSON.parse(fs.readFileSync(HOOKS_SCHEMA_PATH, 'utf-8')); - const ajv = new Ajv({ allErrors: true }); - const validate = ajv.compile(schema); - const valid = validate(data); - if (!valid) { - for (const err of validate.errors) { - console.error(`ERROR: hooks.json schema: ${err.instancePath || '/'} ${err.message}`); + // Without a sidecar, hooks.json keeps its legacy inline ids. With one, the + // sidecar is the sole owner of id/description and hooks.json must stay + // within Claude Code's schema. + let metadata = null; + const metadataPath = metadataPathFor(HOOKS_FILE); + if (fs.existsSync(metadataPath)) { + try { + metadata = JSON.parse(fs.readFileSync(metadataPath, 'utf-8')); + } catch (e) { + console.error(`ERROR: Invalid JSON in ${METADATA_FILENAME}: ${e.message}`); + process.exit(1); + } + + if (validateHarnessCompatibility(data)) { + process.exit(1); + } + + if (UPDATE_FINGERPRINTS) { + try { + metadata = withRefreshedFingerprints(data, metadata); + } catch (error) { + console.error(`ERROR: ${error.message}`); + process.exit(1); + } + } + + if (validateAgainstSchema(metadata, METADATA_SCHEMA_PATH, METADATA_FILENAME)) { + process.exit(1); + } + + const mismatches = findMetadataMismatches(data, metadata); + if (mismatches.length > 0) { + for (const mismatch of mismatches) { + console.error(`ERROR: ${mismatch}`); } process.exit(1); } + + // Validate the merged view so the id/description rules below still apply. + data = applyHooksMetadata(data, metadata); + } + + // Validate against JSON schema + if (validateAgainstSchema(data, HOOKS_SCHEMA_PATH, 'hooks.json')) { + process.exit(1); } // Support both object format { hooks: {...} } and array format @@ -254,6 +400,11 @@ function validateHooks() { process.exit(1); } + if (UPDATE_FINGERPRINTS && metadata) { + fs.writeFileSync(metadataPath, `${JSON.stringify(metadata, null, 2)}\n`); + console.log(`Updated fingerprints in ${METADATA_FILENAME}`); + } + console.log(`Validated ${totalMatchers} hook matchers`); } diff --git a/scripts/dashboard-web.js b/scripts/dashboard-web.js index 044a20fd7..5524853bd 100644 --- a/scripts/dashboard-web.js +++ b/scripts/dashboard-web.js @@ -19,6 +19,7 @@ const { isAllowedOrigin, } = require('./lib/loopback-guard'); const { normalizeAgentTools } = require('./lib/agent-tools'); +const { readHooksConfig } = require('./lib/hooks-config'); const DEFAULT_HOST = '127.0.0.1'; @@ -129,7 +130,9 @@ function loadHooks(_root) { const hooksPath = path.join(root, 'hooks', 'hooks.json'); if (!fs.existsSync(hooksPath)) return []; try { - const data = JSON.parse(fs.readFileSync(hooksPath, 'utf8')); + // Ids and descriptions live in hooks/hooks.metadata.json so that hooks.json + // stays within the key set Claude Code's hooks schema accepts. + const data = readHooksConfig(hooksPath); const hooks = []; for (const [eventName, entries] of Object.entries(data.hooks || {})) { for (const entry of entries || []) { diff --git a/scripts/lib/hooks-config.js b/scripts/lib/hooks-config.js new file mode 100644 index 000000000..11515c4bc --- /dev/null +++ b/scripts/lib/hooks-config.js @@ -0,0 +1,318 @@ +'use strict'; + +/** + * Read hooks/hooks.json together with its sibling hooks/hooks.metadata.json. + * + * Claude Code validates a plugin's hooks.json against its own schema and warns + * about every key it does not recognise, so ECC's stable matcher ids and + * human-readable descriptions cannot live in that file. They are kept in a + * sidecar keyed by event name and aligned with hooks.json entry order, and + * merged back here so the rest of ECC keeps seeing one object with `id` and + * `description` on each matcher entry. + * + * Index alignment alone cannot tell a reordered hooks.json from a correct one, + * so every sidecar entry also carries a fingerprint of the matcher entry it + * describes. A mismatch means the two files drifted apart. + */ + +const crypto = require('crypto'); +const fs = require('fs'); +const path = require('path'); + +const HOOKS_FILENAME = 'hooks.json'; +const METADATA_FILENAME = 'hooks.metadata.json'; +const FINGERPRINT_LENGTH = 12; +const FINGERPRINT_PATTERN = /^[0-9a-f]{12}$/; + +function readJsonObject(filePath, label) { + let raw; + try { + raw = fs.readFileSync(filePath, 'utf8'); + } catch (error) { + throw new Error(`Unable to read ${label} at ${filePath}: ${error.message}`); + } + + let parsed; + try { + parsed = JSON.parse(raw); + } catch (error) { + throw new Error(`Invalid JSON in ${label} at ${filePath}: ${error.message}`); + } + + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new Error(`Invalid ${label} at ${filePath}: expected a JSON object`); + } + + return parsed; +} + +function metadataPathFor(hooksPath) { + return path.join(path.dirname(hooksPath), METADATA_FILENAME); +} + +/** + * JSON.stringify with object keys sorted, so a fingerprint does not change when + * someone reorders the keys inside a hook object. + */ +function stableStringify(value) { + if (Array.isArray(value)) { + return `[${value.map(stableStringify).join(',')}]`; + } + if (value && typeof value === 'object') { + return `{${Object.keys(value).sort().map( + key => `${JSON.stringify(key)}:${stableStringify(value[key])}` + ).join(',')}}`; + } + return JSON.stringify(value); +} + +/** + * Fingerprint the parts of a hooks.json matcher entry that identify it: its + * matcher and its hook commands. Ids and descriptions are excluded so the + * fingerprint is the same whether or not metadata has been merged in. + * + * @param {object} entry - A matcher entry from hooks.json. + * @returns {string} short hex digest + */ +function fingerprintHookEntry(entry) { + const subject = { + matcher: entry && 'matcher' in entry ? entry.matcher : null, + hooks: entry && Array.isArray(entry.hooks) ? entry.hooks : [], + }; + return crypto.createHash('sha256') + .update(stableStringify(subject)) + .digest('hex') + .slice(0, FINGERPRINT_LENGTH); +} + +function eventsOf(hooksConfig) { + return hooksConfig && typeof hooksConfig.hooks === 'object' && hooksConfig.hooks + && !Array.isArray(hooksConfig.hooks) + ? hooksConfig.hooks + : null; +} + +function metadataEntriesOf(metadata) { + return metadata && typeof metadata.entries === 'object' && metadata.entries + && !Array.isArray(metadata.entries) + ? metadata.entries + : null; +} + +/** + * Merge sidecar metadata into a parsed hooks.json object. + * + * Neither argument is mutated; the returned config shares untouched matcher + * entries with the input and copies the ones that receive metadata. + * + * @param {object} hooksConfig - Parsed hooks.json. + * @param {object|null} metadata - Parsed hooks.metadata.json, or null when absent. + * @returns {object} a new hooks configuration with id/description restored. + */ +function applyHooksMetadata(hooksConfig, metadata) { + const events = eventsOf(hooksConfig); + const entriesByEvent = metadataEntriesOf(metadata); + if (!events || !entriesByEvent) { + return hooksConfig; + } + + const mergedEvents = {}; + for (const [event, entries] of Object.entries(events)) { + const eventMetadata = entriesByEvent[event]; + if (!Array.isArray(entries) || !Array.isArray(eventMetadata)) { + mergedEvents[event] = entries; + continue; + } + + mergedEvents[event] = entries.map((entry, index) => { + const entryMetadata = eventMetadata[index]; + if (!entry || typeof entry !== 'object') return entry; + if (!entryMetadata || typeof entryMetadata !== 'object') return entry; + + const merged = { ...entry }; + if (typeof entryMetadata.id === 'string' && !('id' in entry)) { + merged.id = entryMetadata.id; + } + if (typeof entryMetadata.description === 'string' && !('description' in entry)) { + merged.description = entryMetadata.description; + } + return merged; + }); + } + + return { ...hooksConfig, hooks: mergedEvents }; +} + +/** + * Report entries whose metadata is missing, misaligned, or bound to a + * different matcher entry than the one at the same index. + * + * @param {object} hooksConfig - Parsed hooks.json. + * @param {object|null} metadata - Parsed hooks.metadata.json. + * @returns {string[]} human-readable problems; empty when the sidecar lines up. + */ +function findMetadataMismatches(hooksConfig, metadata) { + const problems = []; + const idLocations = new Map(); + const events = eventsOf(hooksConfig) || {}; + const entriesByEvent = metadataEntriesOf(metadata) || {}; + + for (const [event, entries] of Object.entries(events)) { + if (!Array.isArray(entries)) continue; + const eventMetadata = entriesByEvent[event]; + + if (!Array.isArray(eventMetadata)) { + problems.push(`${METADATA_FILENAME} is missing entries for event "${event}"`); + continue; + } + if (eventMetadata.length !== entries.length) { + problems.push( + `${METADATA_FILENAME} lists ${eventMetadata.length} entr(ies) for event "${event}" ` + + `but ${HOOKS_FILENAME} has ${entries.length}` + ); + continue; + } + + eventMetadata.forEach((entry, index) => { + const label = `${METADATA_FILENAME} ${event}[${index}]`; + if (!entry || typeof entry !== 'object') { + problems.push(`${label} is not an object`); + return; + } + if (typeof entry.id !== 'string' || entry.id.trim() === '') { + problems.push(`${label} is missing a non-empty "id"`); + } else if (idLocations.has(entry.id)) { + problems.push(`${label} has duplicate id "${entry.id}" already used by ${idLocations.get(entry.id)}`); + } else { + idLocations.set(entry.id, label); + } + if ('description' in entry && typeof entry.description !== 'string') { + problems.push(`${label} has a non-string "description"`); + } + if (typeof entry.fingerprint !== 'string' || !FINGERPRINT_PATTERN.test(entry.fingerprint)) { + problems.push(`${label} is missing a valid "fingerprint"`); + return; + } + const expected = fingerprintHookEntry(entries[index]); + if (entry.fingerprint !== expected) { + problems.push( + `${label} (id "${entry.id}") fingerprint ${entry.fingerprint} does not match ` + + `${HOOKS_FILENAME} ${event}[${index}] (${expected}); the entries were reordered ` + + 'or the hook command changed - regenerate with ' + + 'node scripts/ci/validate-hooks.js --update-fingerprints' + ); + } + }); + } + + for (const event of Object.keys(entriesByEvent)) { + if (!Array.isArray(events[event])) { + problems.push(`${METADATA_FILENAME} describes event "${event}" which ${HOOKS_FILENAME} does not define`); + } + } + + return problems; +} + +/** + * Return a copy of the sidecar with every fingerprint recomputed from the + * matcher entry at the same index. Used to refresh the sidecar after hook + * commands change. + * + * @param {object} hooksConfig - Parsed hooks.json. + * @param {object} metadata - Parsed hooks.metadata.json. + * @returns {object} a new metadata object + */ +function withRefreshedFingerprints(hooksConfig, metadata) { + const events = eventsOf(hooksConfig) || {}; + const entriesByEvent = metadataEntriesOf(metadata) || {}; + const refreshed = {}; + + // A known fingerprint at another position signals a reorder, not a command + // edit. Require the author to move its metadata before refreshing anything. + const positions = new Map(); + for (const [event, entries] of Object.entries(events)) { + if (!Array.isArray(entries)) continue; + entries.forEach((entry, index) => { + const fingerprint = fingerprintHookEntry(entry); + const locations = positions.get(fingerprint) || []; + positions.set(fingerprint, [...locations, `${event}[${index}]`]); + }); + } + for (const [event, entries] of Object.entries(entriesByEvent)) { + if (!Array.isArray(entries)) continue; + entries.forEach((entry, index) => { + const locations = positions.get(entry?.fingerprint); + const location = `${event}[${index}]`; + if (locations && !locations.includes(location)) { + throw new Error(`Metadata reorder detected at ${location}; move the matching sidecar entry before refreshing fingerprints`); + } + }); + } + + for (const [event, eventMetadata] of Object.entries(entriesByEvent)) { + const entries = Array.isArray(events[event]) ? events[event] : []; + refreshed[event] = Array.isArray(eventMetadata) + ? eventMetadata.map((entry, index) => ( + entry && typeof entry === 'object' && index < entries.length + ? { ...entry, fingerprint: fingerprintHookEntry(entries[index]) } + : entry + )) + : eventMetadata; + } + + return { ...metadata, entries: refreshed }; +} + +function assertMetadataAligned(hooksConfig, metadata, hooksPath) { + const mismatches = findMetadataMismatches(hooksConfig, metadata); + if (mismatches.length > 0) { + throw new Error( + `${METADATA_FILENAME} does not line up with ${hooksPath}:\n ${mismatches.join('\n ')}` + ); + } +} + +/** + * Merge a sidecar into a hooks config, rejecting a sidecar that does not line + * up. Shared by the readers below so a truncated or reordered sidecar fails + * loudly instead of producing entries with the wrong or missing ids. + * + * @param {object} hooksConfig - Parsed hooks.json. + * @param {object} metadata - Parsed hooks.metadata.json. + * @param {string} hooksPath - Used in the error message. + * @returns {object} a new merged hooks configuration + */ +function mergeHooksMetadata(hooksConfig, metadata, hooksPath = HOOKS_FILENAME) { + assertMetadataAligned(hooksConfig, metadata, hooksPath); + return applyHooksMetadata(hooksConfig, metadata); +} + +/** + * Read hooks.json and return it with sidecar metadata merged in. + * + * @param {string} hooksPath - Path to hooks/hooks.json. + * @param {string} [label] - Label used in error messages. + * @returns {object} the merged hooks configuration. + */ +function readHooksConfig(hooksPath, label = HOOKS_FILENAME) { + const hooksConfig = readJsonObject(hooksPath, label); + const metadataPath = metadataPathFor(hooksPath); + if (!fs.existsSync(metadataPath)) { + return hooksConfig; + } + return mergeHooksMetadata(hooksConfig, readJsonObject(metadataPath, METADATA_FILENAME), hooksPath); +} + +module.exports = { + HOOKS_FILENAME, + METADATA_FILENAME, + applyHooksMetadata, + findMetadataMismatches, + fingerprintHookEntry, + mergeHooksMetadata, + metadataPathFor, + readHooksConfig, + readJsonObject, + withRefreshedFingerprints, +}; diff --git a/scripts/lib/install-lifecycle.js b/scripts/lib/install-lifecycle.js index 99ec19614..ec9cb1d80 100644 --- a/scripts/lib/install-lifecycle.js +++ b/scripts/lib/install-lifecycle.js @@ -36,6 +36,7 @@ const { adaptAntigravityAgent } = require('./install/antigravity-agent'); const { buildInstallIndex, rewriteRelativeLinks } = require('./install/link-rewrite'); const { getInstallTargetAdapter, listInstallTargetAdapters } = require('./install-targets/registry'); const { resolveInvocationEnvironment } = require('./invocation-environment'); +const { mergeHooksMetadata, metadataPathFor } = require('./hooks-config'); const OPENCODE_BUILD_ARTIFACT = path.join('.opencode', 'dist'); const OPENCODE_BUILD_SCRIPT = path.join('scripts', 'build-opencode.js'); const OPENCODE_PLUGIN_NOT_BUILT_CODE = 'opencode-plugin-not-built'; @@ -535,6 +536,25 @@ function readJsonNoFollow(filePath) { return JSON.parse(readFileNoFollow(filePath, 'utf8')); } +/** + * Read hooks.json and merge in hooks/hooks.metadata.json without following + * symlinks. The sidecar holds the stable matcher ids that hooks.json cannot + * carry, because Claude Code reports unknown keys when the plugin loads. A + * sidecar that does not line up with hooks.json is rejected before repair can + * reconcile matchers under the wrong ids. + * + * @param {string} hooksPath - Path to the source hooks.json. + * @returns {object} the hooks configuration with ids and descriptions restored. + */ +function readHooksConfigNoFollow(hooksPath) { + const hooksConfig = readJsonNoFollow(hooksPath); + const metadataPath = metadataPathFor(hooksPath); + if (!fs.existsSync(metadataPath)) { + return hooksConfig; + } + return mergeHooksMetadata(hooksConfig, readJsonNoFollow(metadataPath), hooksPath); +} + function assertClaudeSettingsDestination(operation, trustedRoot, target = null) { if (target && target !== 'claude' && target !== 'claude-project') { throw new Error('Refusing to manage Claude hooks for a non-Claude target.'); @@ -720,7 +740,7 @@ function hydrateRecordedOperations(repoRoot, operations, trustedRoot) { sourcePath, previousManagedHooks: operation.managedHooks, managedHooks: materializeManagedHooks( - readJsonNoFollow(sourcePath), + readHooksConfigNoFollow(sourcePath), trustedRoot ), }; diff --git a/scripts/lib/install-targets/helpers.js b/scripts/lib/install-targets/helpers.js index dbb5b44e5..f69d75e86 100644 --- a/scripts/lib/install-targets/helpers.js +++ b/scripts/lib/install-targets/helpers.js @@ -5,6 +5,7 @@ const { CLAUDE_HOOKS_CONFIG_PATH, getClaudeSettingsPath, } = require('../install/claude-settings'); +const { METADATA_FILENAME } = require('../hooks-config'); const PLATFORM_SOURCE_PATH_OWNERS = Object.freeze({ '.claude-plugin': 'claude', @@ -176,7 +177,9 @@ function planClaudeHooksOperations(adapter, module, input) { return [ ...operations, ...fs.readdirSync(sourceHooksRoot, { withFileTypes: true }) - .filter(entry => entry.name !== 'hooks.json') + // hooks.json is merged into settings.json above, and its metadata sidecar + // is consumed with it, so neither is scaffolded into the target hooks dir. + .filter(entry => entry.name !== 'hooks.json' && entry.name !== METADATA_FILENAME) .sort((left, right) => left.name.localeCompare(right.name)) .map(entry => adapter.createScaffoldOperation( module.id, diff --git a/scripts/lib/install/plan.js b/scripts/lib/install/plan.js index 08173a672..1400557c7 100644 --- a/scripts/lib/install/plan.js +++ b/scripts/lib/install/plan.js @@ -7,6 +7,7 @@ const { execFileSync } = require('child_process'); const { resolveInstallPlan } = require('../install-manifests'); const { getInstallTargetAdapter } = require('../install-targets/registry'); const { resolveInvocationEnvironment } = require('../invocation-environment'); +const { readHooksConfig } = require('../hooks-config'); const { materializeManagedHooks, } = require('./claude-settings'); @@ -136,7 +137,9 @@ function materializeClaudeSettingsOperation(sourceRoot, operation) { return []; } - const hooksConfig = readJsonObject(sourcePath, operation.sourceRelativePath); + // Stable ids and descriptions live in hooks/hooks.metadata.json; readHooksConfig + // merges them back so managed settings entries keep their ids. + const hooksConfig = readHooksConfig(sourcePath, operation.sourceRelativePath); const managedHooks = materializeManagedHooks( hooksConfig, path.dirname(operation.destinationPath) diff --git a/tests/hooks/continuous-learning-observe-runner.test.js b/tests/hooks/continuous-learning-observe-runner.test.js index 37ca3ce31..9445a3c1d 100644 --- a/tests/hooks/continuous-learning-observe-runner.test.js +++ b/tests/hooks/continuous-learning-observe-runner.test.js @@ -17,6 +17,7 @@ const hooksJsonPath = path.join(repoRoot, 'hooks', 'hooks.json'); const runWithFlagsPath = path.join(repoRoot, 'scripts', 'hooks', 'run-with-flags.js'); const observeRunner = require(path.join(repoRoot, 'scripts', 'hooks', 'observe-runner.js')); const postToolUseDispatcher = require(path.join(repoRoot, 'scripts', 'hooks', 'posttooluse-dispatcher.js')); +const { readHooksConfig } = require(path.join(repoRoot, 'scripts', 'lib', 'hooks-config.js')); function test(name, fn) { try { @@ -31,7 +32,7 @@ function test(name, fn) { } function loadHook(id) { - const hookGroups = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')).hooks; + const hookGroups = readHooksConfig(hooksJsonPath).hooks; const hooks = Object.values(hookGroups).flat(); const hook = hooks.find(candidate => candidate.id === id); assert.ok(hook, `Expected ${id} in hooks/hooks.json`); diff --git a/tests/hooks/hooks-metadata.test.js b/tests/hooks/hooks-metadata.test.js new file mode 100644 index 000000000..5294d91fe --- /dev/null +++ b/tests/hooks/hooks-metadata.test.js @@ -0,0 +1,293 @@ +/** + * Tests for the hooks.json / hooks.metadata.json split. + * + * Claude Code validates a plugin's hooks.json against its own schema and prints + * every key it does not recognise when the plugin loads. These tests keep the + * unknown keys out of hooks.json and keep the sidecar aligned with it. + * + * Run with: node tests/hooks/hooks-metadata.test.js + */ + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const { + applyHooksMetadata, + findMetadataMismatches, + fingerprintHookEntry, + metadataPathFor, + readHooksConfig, + withRefreshedFingerprints, +} = require('../../scripts/lib/hooks-config'); + +const REPO_ROOT = path.resolve(__dirname, '../..'); +const HOOKS_PATH = path.join(REPO_ROOT, 'hooks', 'hooks.json'); +const METADATA_PATH = metadataPathFor(HOOKS_PATH); + +function readJson(filePath) { + return JSON.parse(fs.readFileSync(filePath, 'utf8')); +} + +function eachMatcher(hooksConfig, visit) { + for (const [event, entries] of Object.entries(hooksConfig.hooks || {})) { + (entries || []).forEach((entry, index) => visit(entry, `${event}[${index}]`)); + } +} + +const tests = []; +function test(name, fn) { + tests.push({ name, fn }); +} + +test('hooks.json does not declare $schema', () => { + const hooksConfig = readJson(HOOKS_PATH); + assert.ok( + !('$schema' in hooksConfig), + 'hooks.json must not define "$schema" - Claude Code reports it as an unknown key' + ); +}); + +test('hooks.json matcher entries carry no id or description', () => { + const hooksConfig = readJson(HOOKS_PATH); + eachMatcher(hooksConfig, (entry, label) => { + assert.ok(!('id' in entry), `${label} must not define "id" - it belongs in hooks.metadata.json`); + assert.ok( + !('description' in entry), + `${label} must not define "description" - it belongs in hooks.metadata.json` + ); + }); +}); + +test('metadata sidecar exists and lines up with hooks.json', () => { + assert.ok(fs.existsSync(METADATA_PATH), 'hooks/hooks.metadata.json is missing'); + const mismatches = findMetadataMismatches(readJson(HOOKS_PATH), readJson(METADATA_PATH)); + assert.deepStrictEqual(mismatches, [], `metadata is misaligned:\n${mismatches.join('\n')}`); +}); + +test('every matcher entry has a unique id after merging', () => { + const merged = readHooksConfig(HOOKS_PATH); + const seen = new Map(); + let count = 0; + + eachMatcher(merged, (entry, label) => { + count += 1; + assert.ok( + typeof entry.id === 'string' && entry.id.trim() !== '', + `${label} has no id after merging metadata` + ); + assert.ok(!seen.has(entry.id), `duplicate id "${entry.id}" at ${label} and ${seen.get(entry.id)}`); + seen.set(entry.id, label); + }); + + assert.ok(count > 0, 'expected at least one matcher entry'); +}); + +test('merging leaves hook commands untouched', () => { + const raw = readJson(HOOKS_PATH); + const merged = readHooksConfig(HOOKS_PATH); + + const commandsOf = config => Object.entries(config.hooks || {}).flatMap(([event, entries]) => ( + (entries || []).flatMap((entry, index) => (entry.hooks || []).map( + (hook, hookIndex) => `${event}[${index}].hooks[${hookIndex}]:${JSON.stringify(hook)}` + )) + )); + + assert.deepStrictEqual(commandsOf(merged), commandsOf(raw)); +}); + +test('applyHooksMetadata does not overwrite an id already present', () => { + const hooksConfig = { hooks: { PreToolUse: [{ id: 'existing', matcher: 'Bash', hooks: [] }] } }; + const merged = applyHooksMetadata(hooksConfig, { entries: { PreToolUse: [{ id: 'from-sidecar' }] } }); + assert.strictEqual(merged.hooks.PreToolUse[0].id, 'existing'); +}); + +test('applyHooksMetadata returns a new config and leaves its inputs untouched', () => { + const entry = { matcher: 'Bash', hooks: [{ type: 'command', command: 'node a.js' }] }; + const hooksConfig = { hooks: { PreToolUse: [entry] } }; + const metadata = { entries: { PreToolUse: [{ id: 'a', description: 'A' }] } }; + + const merged = applyHooksMetadata(hooksConfig, metadata); + + assert.notStrictEqual(merged, hooksConfig); + assert.notStrictEqual(merged.hooks.PreToolUse[0], entry); + assert.deepStrictEqual(merged.hooks.PreToolUse[0], { ...entry, id: 'a', description: 'A' }); + assert.deepStrictEqual(hooksConfig, { hooks: { PreToolUse: [entry] } }); + assert.ok(!('id' in entry) && !('description' in entry), 'input entry must not be mutated'); + assert.strictEqual(merged.hooks.PreToolUse[0].hooks, entry.hooks, 'untouched nested data is shared'); +}); + +const alpha = { matcher: 'Bash', hooks: [{ type: 'command', command: 'node alpha.js' }] }; +const beta = { matcher: 'Bash', hooks: [{ type: 'command', command: 'node beta.js' }] }; +const alphaMeta = { id: 'a', fingerprint: fingerprintHookEntry(alpha) }; +const betaMeta = { id: 'b', fingerprint: fingerprintHookEntry(beta) }; + +test('findMetadataMismatches reports length and coverage problems', () => { + const hooksConfig = { hooks: { PreToolUse: [alpha, beta] } }; + + assert.strictEqual(findMetadataMismatches(hooksConfig, { entries: {} }).length, 1); + assert.strictEqual( + findMetadataMismatches(hooksConfig, { entries: { PreToolUse: [alphaMeta] } }).length, + 1 + ); + assert.strictEqual( + findMetadataMismatches(hooksConfig, { + entries: { PreToolUse: [alphaMeta, { ...betaMeta, id: '' }] }, + }).length, + 1 + ); + assert.strictEqual( + findMetadataMismatches(hooksConfig, { + entries: { PreToolUse: [alphaMeta, { ...betaMeta, description: 1 }] }, + }).length, + 1 + ); + assert.strictEqual( + findMetadataMismatches(hooksConfig, { + entries: { PreToolUse: [alphaMeta, betaMeta], Stop: [] }, + }).length, + 1 + ); + assert.deepStrictEqual( + findMetadataMismatches(hooksConfig, { entries: { PreToolUse: [alphaMeta, betaMeta] } }), + [] + ); +}); + +test('findMetadataMismatches detects reordered entries and missing fingerprints', () => { + const hooksConfig = { hooks: { PreToolUse: [alpha, beta] } }; + + const reordered = findMetadataMismatches(hooksConfig, { entries: { PreToolUse: [betaMeta, alphaMeta] } }); + assert.strictEqual(reordered.length, 2, 'each swapped entry is reported'); + assert.match(reordered[0], /PreToolUse\[0\] \(id "b"\) fingerprint .* does not match/); + + const changed = findMetadataMismatches( + { hooks: { PreToolUse: [alpha, { ...beta, matcher: 'Write' }] } }, + { entries: { PreToolUse: [alphaMeta, betaMeta] } } + ); + assert.strictEqual(changed.length, 1, 'a changed matcher invalidates the fingerprint'); + + const missing = findMetadataMismatches(hooksConfig, { + entries: { PreToolUse: [{ id: 'a' }, { id: 'b', fingerprint: 'nope' }] }, + }); + assert.strictEqual(missing.length, 2); + assert.match(missing[0], /missing a valid "fingerprint"/); +}); + +test('fingerprintHookEntry ignores id, description, and key order', () => { + const base = fingerprintHookEntry(alpha); + assert.match(base, /^[0-9a-f]{12}$/); + assert.strictEqual(fingerprintHookEntry({ ...alpha, id: 'x', description: 'y' }), base); + assert.strictEqual( + fingerprintHookEntry({ hooks: [{ command: 'node alpha.js', type: 'command' }], matcher: 'Bash' }), + base + ); + assert.notStrictEqual(fingerprintHookEntry(beta), base); +}); + +test('withRefreshedFingerprints rewrites fingerprints without touching ids', () => { + const hooksConfig = { hooks: { PreToolUse: [alpha, beta] } }; + const stale = { + $schema: 's', + entries: { PreToolUse: [{ id: 'a', fingerprint: '000000000000' }, { id: 'b' }] }, + }; + + const refreshed = withRefreshedFingerprints(hooksConfig, stale); + + assert.deepStrictEqual(refreshed, { $schema: 's', entries: { PreToolUse: [alphaMeta, betaMeta] } }); + assert.deepStrictEqual(findMetadataMismatches(hooksConfig, refreshed), []); + assert.strictEqual(stale.entries.PreToolUse[0].fingerprint, '000000000000', 'input is not mutated'); +}); + +test('readHooksConfig rejects a sidecar that does not line up', () => { + const tempDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'ecc-hooks-')); + const tempHooks = path.join(tempDir, 'hooks.json'); + fs.writeFileSync(tempHooks, JSON.stringify({ hooks: { PreToolUse: [alpha, beta] } })); + fs.writeFileSync( + metadataPathFor(tempHooks), + JSON.stringify({ entries: { PreToolUse: [betaMeta, alphaMeta] } }) + ); + + try { + assert.throws(() => readHooksConfig(tempHooks), /does not line up with .*hooks\.json[\s\S]*fingerprint/); + + fs.writeFileSync( + metadataPathFor(tempHooks), + JSON.stringify({ entries: { PreToolUse: [alphaMeta, betaMeta] } }) + ); + const merged = readHooksConfig(tempHooks); + assert.deepStrictEqual(merged.hooks.PreToolUse.map(entry => entry.id), ['a', 'b']); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } +}); + +test('readHooksConfig returns raw config when the sidecar is absent', () => { + const tempDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'ecc-hooks-')); + const tempHooks = path.join(tempDir, 'hooks.json'); + fs.writeFileSync(tempHooks, JSON.stringify({ hooks: { Stop: [{ hooks: [] }] } })); + + try { + const config = readHooksConfig(tempHooks); + assert.deepStrictEqual(config, { hooks: { Stop: [{ hooks: [] }] } }); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } +}); + +test('failed refresh validation preserves the original sidecar bytes', () => { + const root = fs.mkdtempSync(path.join(require('os').tmpdir(), 'ecc-metadata-refresh-')); + try { + for (const relative of ['scripts/ci/validate-hooks.js', 'scripts/lib/hooks-config.js', + 'schemas/hooks.schema.json', 'schemas/hooks-metadata.schema.json']) { + const destination = path.join(root, relative); + fs.mkdirSync(path.dirname(destination), { recursive: true }); + fs.copyFileSync(path.join(REPO_ROOT, relative), destination); + } + fs.mkdirSync(path.join(root, 'hooks')); + fs.writeFileSync(path.join(root, 'hooks/hooks.json'), JSON.stringify({ hooks: { PreToolUse: [alpha] } })); + const sidecar = path.join(root, 'hooks/hooks.metadata.json'); + const original = JSON.stringify({ entries: { PreToolUse: [{ ...alphaMeta, id: '', fingerprint: '000000000000' }] } }); + fs.writeFileSync(sidecar, original); + const result = require('child_process').spawnSync(process.execPath, + [path.join(root, 'scripts/ci/validate-hooks.js'), '--update-fingerprints'], { + encoding: 'utf8', env: { ...process.env, NODE_PATH: path.join(REPO_ROOT, 'node_modules') }, + }); + assert.strictEqual(result.status, 1, result.stderr); + assert.match(result.stderr, /id|non-empty/); + assert.strictEqual(fs.readFileSync(sidecar, 'utf8'), original); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('refresh refuses reordered hooks instead of rebinding stable ids', () => { + const config = { hooks: { PreToolUse: [beta, alpha] } }; + const metadata = { entries: { PreToolUse: [alphaMeta, betaMeta] } }; + assert.throws(() => withRefreshedFingerprints(config, metadata), /reorder/i); + assert.deepStrictEqual(metadata.entries.PreToolUse, [alphaMeta, betaMeta]); +}); + +test('alignment rejects duplicate ids across events', () => { + const config = { hooks: { PreToolUse: [alpha], PostToolUse: [beta] } }; + const metadata = { entries: { + PreToolUse: [alphaMeta], PostToolUse: [{ ...betaMeta, id: alphaMeta.id }], + } }; + assert.ok(findMetadataMismatches(config, metadata).some(problem => + /duplicate/.test(problem) && /PreToolUse/.test(problem) && /PostToolUse/.test(problem))); +}); + +let failures = 0; +for (const { name, fn } of tests) { + try { + fn(); + console.log(` PASS ${name}`); + } catch (error) { + failures += 1; + console.error(` FAIL ${name}`); + console.error(` ${error.message}`); + } +} + +console.log(`\nResults: Passed: ${tests.length - failures}, Failed: ${failures}`); +process.exit(failures === 0 ? 0 : 1); diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index 93ad6133b..635566b27 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -9,6 +9,7 @@ const path = require('path'); const fs = require('fs'); const os = require('os'); const { execFileSync, spawn, spawnSync } = require('child_process'); +const { readHooksConfig } = require('../../scripts/lib/hooks-config'); const SKIP_BASH = process.platform === 'win32'; @@ -2573,7 +2574,7 @@ async function runTests() { if ( test('hooks.json consolidates PreToolUse Bash and all PostToolUse hooks', () => { const hooksPath = path.join(__dirname, '..', '..', 'hooks', 'hooks.json'); - const hooks = JSON.parse(fs.readFileSync(hooksPath, 'utf8')); + const hooks = readHooksConfig(hooksPath); const preBash = hooks.hooks.PreToolUse.filter(entry => entry.matcher === 'Bash'); const postEntries = hooks.hooks.PostToolUse; @@ -2602,7 +2603,7 @@ async function runTests() { if ( test('hooks.json gives PowerShell dedicated GateGuard and governance routes', () => { const hooksPath = path.join(__dirname, '..', '..', 'hooks', 'hooks.json'); - const hooks = JSON.parse(fs.readFileSync(hooksPath, 'utf8')); + const hooks = readHooksConfig(hooksPath); const powerShellRoutes = hooks.hooks.PreToolUse.filter(entry => entry.matcher === 'PowerShell'); const governanceRoute = hooks.hooks.PreToolUse.find(entry => entry.id === 'pre:governance-capture'); @@ -2641,7 +2642,7 @@ async function runTests() { if ( test('configured PowerShell routes enforce denial and emit redacted governance evidence', () => { const root = path.join(__dirname, '..', '..'); - const hooks = JSON.parse(fs.readFileSync(path.join(root, 'hooks', 'hooks.json'), 'utf8')); + const hooks = readHooksConfig(path.join(root, 'hooks', 'hooks.json')); const gateRoute = hooks.hooks.PreToolUse.find(entry => entry.id === 'pre:powershell:gateguard-fact-force'); const governanceRoute = hooks.hooks.PreToolUse.find(entry => entry.id === 'pre:governance-capture'); const stateDir = createTestDir(); diff --git a/tests/hooks/posttooluse-dispatcher.test.js b/tests/hooks/posttooluse-dispatcher.test.js index c21f003f3..ae6dbaaa1 100644 --- a/tests/hooks/posttooluse-dispatcher.test.js +++ b/tests/hooks/posttooluse-dispatcher.test.js @@ -13,6 +13,7 @@ const { spawnSync } = require('child_process'); const repoRoot = path.join(__dirname, '..', '..'); const hooksPath = path.join(repoRoot, 'hooks', 'hooks.json'); const dispatcherPath = path.join(repoRoot, 'scripts', 'hooks', 'posttooluse-dispatcher.js'); +const { readHooksConfig } = require(path.join(repoRoot, 'scripts', 'lib', 'hooks-config.js')); function test(name, fn) { try { @@ -78,7 +79,7 @@ function runTests() { if ( test('hooks.json exposes one sync and one async PostToolUse entry', () => { - const entries = JSON.parse(fs.readFileSync(hooksPath, 'utf8')).hooks.PostToolUse; + const entries = readHooksConfig(hooksPath).hooks.PostToolUse; assert.strictEqual(entries.length, 2, 'PostToolUse should launch at most two commands'); assert.deepStrictEqual( entries.map(entry => entry.id), @@ -162,7 +163,7 @@ function runTests() { if ( test('actual hooks.json commands preserve Edit dry-run output and IDs', () => { - const entries = JSON.parse(fs.readFileSync(hooksPath, 'utf8')).hooks.PostToolUse; + const entries = readHooksConfig(hooksPath).hooks.PostToolUse; const raw = JSON.stringify({ hook_event_name: 'PostToolUse', tool_name: 'Edit', @@ -194,7 +195,7 @@ function runTests() { if ( test('actual hooks.json commands never echo truncated oversized input', () => { - const entries = JSON.parse(fs.readFileSync(hooksPath, 'utf8')).hooks.PostToolUse; + const entries = readHooksConfig(hooksPath).hooks.PostToolUse; const values = ['x'.repeat(1024 * 1024 + 1024), 'é'.repeat(600000), '\u{1F600}'.repeat(300000)]; for (const value of values) { @@ -270,7 +271,7 @@ function runTests() { if ( test('public dispatcher IDs disable their complete phase', () => { - const entries = JSON.parse(fs.readFileSync(hooksPath, 'utf8')).hooks.PostToolUse; + const entries = readHooksConfig(hooksPath).hooks.PostToolUse; const raw = JSON.stringify({ hook_event_name: 'PostToolUse', tool_name: 'Edit', @@ -480,7 +481,7 @@ function runTests() { assert.strictEqual(result.status, 0, result.stderr); assert.strictEqual(result.stdout, '', 'require() alone must not run main() or echo stdin'); - const entries = JSON.parse(fs.readFileSync(hooksPath, 'utf8')).hooks.PostToolUse; + const entries = readHooksConfig(hooksPath).hooks.PostToolUse; assert.ok( entries.every(entry => entry.hooks[0].command.includes('require(s).cli()')), 'hooks.json must invoke the explicit cli() entrypoint' diff --git a/tests/hooks/skill-run-tracker.test.js b/tests/hooks/skill-run-tracker.test.js index bd97b50c8..d3e63e247 100644 --- a/tests/hooks/skill-run-tracker.test.js +++ b/tests/hooks/skill-run-tracker.test.js @@ -16,6 +16,7 @@ const os = require('os'); const path = require('path'); const { buildRecord, deriveOutcome, extractSkillId, run } = require('../../scripts/hooks/skill-run-tracker'); +const { readHooksConfig } = require('../../scripts/lib/hooks-config'); const { MAX_RUN_RECORDS, RUNS_FILE_MODE, @@ -238,9 +239,7 @@ test('an end-to-end Skill hook run lands exactly one non-sensitive record', () = // needs its own hooks.json entry. Without it, hard Skill failures are silently // dropped and the dashboard's success rate is inflated. test('the tracker is registered for PostToolUseFailure so hard failures are recorded', () => { - const hooksConfig = JSON.parse( - fs.readFileSync(path.join(__dirname, '..', '..', 'hooks', 'hooks.json'), 'utf8') - ); + const hooksConfig = readHooksConfig(path.join(__dirname, '..', '..', 'hooks', 'hooks.json')); const entries = (hooksConfig.hooks.PostToolUseFailure || []) .filter(entry => entry.id === 'post:skill:track'); diff --git a/tests/hooks/stop-hooks-stdout.test.js b/tests/hooks/stop-hooks-stdout.test.js index 3d0617c57..5d3efdf86 100644 --- a/tests/hooks/stop-hooks-stdout.test.js +++ b/tests/hooks/stop-hooks-stdout.test.js @@ -24,9 +24,8 @@ const { spawnSync } = require('child_process'); const repoRoot = path.join(__dirname, '..', '..'); const runner = path.join(repoRoot, 'scripts', 'hooks', 'run-with-flags.js'); -const hooksConfig = JSON.parse( - fs.readFileSync(path.join(repoRoot, 'hooks', 'hooks.json'), 'utf8') -); +const { readHooksConfig } = require(path.join(repoRoot, 'scripts', 'lib', 'hooks-config.js')); +const hooksConfig = readHooksConfig(path.join(repoRoot, 'hooks', 'hooks.json')); const MAX_STDIN = 1024 * 1024; const SUBPROCESS_TIMEOUT_MS = process.platform === 'darwin' && process.env.CI === 'true' diff --git a/tests/integration/hooks.test.js b/tests/integration/hooks.test.js index 677e2b952..96ad9b4d1 100644 --- a/tests/integration/hooks.test.js +++ b/tests/integration/hooks.test.js @@ -12,6 +12,7 @@ const path = require('path'); const fs = require('fs'); const os = require('os'); const { spawn } = require('child_process'); +const { readHooksConfig } = require('../../scripts/lib/hooks-config'); const REPO_ROOT = path.join(__dirname, '..', '..'); // Test helper @@ -282,7 +283,7 @@ async function runTests() { const scriptsDir = path.join(__dirname, '..', '..', 'scripts', 'hooks'); const hooksJsonPath = path.join(__dirname, '..', '..', 'hooks', 'hooks.json'); - const hooks = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')); + const hooks = readHooksConfig(hooksJsonPath); // ========================================== // Input Format Tests diff --git a/tests/lib/install-lifecycle.test.js b/tests/lib/install-lifecycle.test.js index ab20e54ca..1eed5071b 100644 --- a/tests/lib/install-lifecycle.test.js +++ b/tests/lib/install-lifecycle.test.js @@ -27,6 +27,7 @@ const { assertClaudeSettingsPath, materializeManagedHooks, } = require('../../scripts/lib/install/claude-settings'); +const { readHooksConfig } = require('../../scripts/lib/hooks-config'); const REPO_ROOT = path.join(__dirname, '..', '..'); const CURRENT_PACKAGE_VERSION = JSON.parse( @@ -158,7 +159,7 @@ function managedHookEntry(id, command) { function currentManagedHooks(targetRoot) { return materializeManagedHooks( - JSON.parse(fs.readFileSync(path.join(REPO_ROOT, 'hooks', 'hooks.json'), 'utf8')), + readHooksConfig(path.join(REPO_ROOT, 'hooks', 'hooks.json')), targetRoot ); } diff --git a/tests/plugin-manifest.test.js b/tests/plugin-manifest.test.js index 8f4ac1ba0..74bd25ec4 100644 --- a/tests/plugin-manifest.test.js +++ b/tests/plugin-manifest.test.js @@ -17,6 +17,7 @@ const assert = require('assert'); const fs = require('fs'); const path = require('path'); +const { readHooksConfig } = require('../scripts/lib/hooks-config'); const repoRoot = path.resolve(__dirname, '..'); const packageJsonPath = path.join(repoRoot, 'package.json'); @@ -394,7 +395,7 @@ test('codex lifecycle hook bundle contains only Codex 0.146-supported schema', ( } } - const claudeConfig = loadJsonObject(path.join(repoRoot, 'hooks', 'hooks.json'), 'hooks/hooks.json'); + const claudeConfig = readHooksConfig(path.join(repoRoot, 'hooks', 'hooks.json'), 'hooks/hooks.json'); const sourceSessionStart = claudeConfig.hooks.SessionStart.find(group => group.id === 'session:start'); const expectedSessionStart = { ...sourceSessionStart, From 1ed03ecf2ec91aac77f3c98094d7d1136e89b4d4 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Sat, 12 Sep 2026 04:17:37 -0400 Subject: [PATCH 12/72] feat(control-pane): live control-plane view with 2D projection and static-threshold advisories Exact-head independent local Codex review PASS with no P0/P1. CI run 34680860653 attempt 2 passed at 0707cd431c2b242200a7ef28ab984f4a2c032238. Includes HTTP/schema failure handling, coalesced sampling cache and regression tests. Disclosed P2 follow-ups remain in the merge-queue receipt. Rollback: revert this squash commit. No deployment or publication claim. --- docs/control-plane/TCAS-HOOK.md | 81 ++++ docs/control-plane/VIEW-CONTRACT.md | 141 +++++++ scripts/lib/agent-proximity/distance.js | 28 +- scripts/lib/agent-proximity/index.js | 3 +- scripts/lib/agent-proximity/projection.js | 305 +++++++++++++++ .../lib/control-pane/control-plane-view-ui.js | 243 ++++++++++++ .../lib/control-pane/control-plane-view.js | 358 ++++++++++++++++++ scripts/lib/control-pane/proximity-viz.js | 1 + scripts/lib/control-pane/proximity.js | 12 + scripts/lib/control-pane/server.js | 43 +++ tests/lib/agent-proximity-projection.test.js | 181 +++++++++ tests/lib/control-plane-view-ui.test.js | 51 +++ tests/lib/control-plane-view.test.js | 334 ++++++++++++++++ tests/scripts/control-pane.test.js | 59 +++ 14 files changed, 1828 insertions(+), 12 deletions(-) create mode 100644 docs/control-plane/TCAS-HOOK.md create mode 100644 docs/control-plane/VIEW-CONTRACT.md create mode 100644 scripts/lib/agent-proximity/projection.js create mode 100644 scripts/lib/control-pane/control-plane-view-ui.js create mode 100644 scripts/lib/control-pane/control-plane-view.js create mode 100644 tests/lib/agent-proximity-projection.test.js create mode 100644 tests/lib/control-plane-view-ui.test.js create mode 100644 tests/lib/control-plane-view.test.js diff --git a/docs/control-plane/TCAS-HOOK.md b/docs/control-plane/TCAS-HOOK.md new file mode 100644 index 000000000..9b9b02c58 --- /dev/null +++ b/docs/control-plane/TCAS-HOOK.md @@ -0,0 +1,81 @@ +# TCAS hook: pre-merge deconfliction (slice b, design) + +Status: design only. Nothing in this document is implemented. Slice (a), the live view and the advisory feed it reads, shipped in `VIEW-CONTRACT.md`. + +## Goal + +Stop two agents from finishing overlapping edits and meeting at the merge. The scan already knows when two working sets converge; the hook is what turns that knowledge into a maneuver inside the harness, before either agent commits. + +Push plan wording: "a PreToolUse/Edit hook that reads the advisory feed and returns steer, pause or wait for the lower-priority agent, logged to the capsule." + +## Inputs + +1. The event feed: `GET /api/control-plane/events` on the local control pane, or the same document written to a file by `scripts/proximity-tick.js --json` for sessions without a pane. Events of kind `proximity.advisory` with `action.type` `transmit` or `steer` and a deterministic `id`. +2. The hook's own session id. Claude Code passes `session_id` on stdin; the ECC session adapter maps it to the ECC2 `sessions.id` the scan uses. Codex and Hermes use the instruction-backed equivalent (see below). +3. The tool call: `tool_name` and `tool_input.file_path` for Edit, Write and MultiEdit. Bash is out of scope for v1. + +## Decision + +For each advisory event whose `subject` includes this session: + +| Event | This session is | Maneuver | Hook result | +|---|---|---|---| +| `traffic`, action `transmit` | either side | **transmit**: inject the other agent's working set as a system message | exit 0, message on stderr (warn, never block) | +| `resolution`, action `steer` | `hold` | **hold**: continue | exit 0, short note | +| `resolution`, action `steer` | `steer`, and `file_path` is in the other agent's working set | **pause**: stop editing that file until the other agent's diff lands | exit 2 with the reason (blocks this one tool call) | +| `resolution`, action `steer` | `steer`, and `file_path` is not in the other agent's working set | **wait**: allowed, but told to keep to non-overlapping files | exit 0, message on stderr | +| `resolution`, action `steer` | `steer`, and a `steer` target exists | **steer**: suggest the disjoint files or subtree the agent should move to | exit 0, message; exit 2 only if the edit is on the shared file | + +The maneuver is deterministic: both agents read the same event, `hold` and `steer` are named in it, so the two sides never pick the same move. This is the TCAS coordination property and it is why the view computes right-of-way once, centrally, rather than each hook deciding. + +`pause` blocks a single tool call, not the session. The agent sees the reason and can pick another file. Blocking is bounded by the event's `at`: an event older than the pane's poll interval times three is stale and the hook does not block on it. + +## Priority + +Right-of-way comes from the event (`action.hold`, `action.steer`). The view computes it as more progress, then earlier start, then stable id (`rightOfWay` in `scripts/lib/agent-proximity/distance.js`). The hook never recomputes it. + +## Logging to the capsule + +Every decision is one entry in the session's capsule journal (`scripts/lib/eval-harness/capsule.js`, hash-linked NDJSON): + +```json +{ + "kind": "tcas.decision", + "event_id": "proximity.advisory:session-a|session-b:resolution", + "session": "session-b", + "tool": "Edit", + "file": "src/api/users.js", + "maneuver": "pause", + "blocked": true, + "risk": 1, + "threshold": { "ta": 0.35, "ra": 0.7, "source": "static" }, + "at": "2026-09-11T20:01:03.000Z" +} +``` + +The capsule is the baseline counter for the 85 percent goal: rebase and merge-conflict triage incidents per week are counted from these entries plus `git rerere` and conflict markers, two weeks before and two weeks after the hook is on. No percentage is claimed before that. + +## Where it plugs in + +- **Claude Code**: a `PreToolUse` entry in `hooks/hooks.json` with matcher `Edit|Write|MultiEdit`, routed through `scripts/hooks/run-with-flags.js` so `ECC_HOOK_PROFILE` and `ECC_DISABLED_HOOKS` gate it. Script under `scripts/hooks/tcas-pre-edit.js`, helpers in `scripts/lib/control-pane/tcas.js`. Budget: under 200 ms, no network beyond loopback, exit 0 on any parse or fetch error. +- **Codex**: no PreToolUse. The instruction-backed equivalent is the `proximity_steer` / `proximity_hold` message the tick already writes into the ECC2 `messages` table, surfaced on the next turn. `pause` degrades to a strong instruction. +- **Hermes**: gateway hook on the tool-call path, same decision table, same capsule entry. + +## Off switch and safety + +- Disabled by default. On with `ECC_TCAS_HOOK=1` or the hook profile. +- Read-only against the pane. It never writes to the sessions or messages tables. +- No lease is acquired. Durable leases are slice (c), the worktree lease table in ecc2 `session/store.rs` next to `messages`; until then a `pause` is a per-call block, not a lock, and two hooks racing on the same file is possible but harmless (both see the same event and the same `steer`). +- Fails open. Any error is exit 0 with a `[TCAS]` line on stderr. + +## Tests to write with it + +- Decision table: one test per row above, driven by a fixture event feed and a stdin payload. +- Staleness: an event older than the window does not block. +- Fail-open: unreachable pane, malformed JSON, missing session id. +- Capsule: one entry per decision, hash chain intact, replay reproduces the same bytes. +- Integration: two fake sessions with overlapping working sets, the lower-priority one gets exit 2 on the shared file and exit 0 on a disjoint file. + +## Out of scope for (b) + +Learned thresholds, closure-rate escalation, mesh mode, cross-machine airspace, the `x_sem`, `x_vec`, `x_freq` channels (slice g), and the lease table (slice c). diff --git a/docs/control-plane/VIEW-CONTRACT.md b/docs/control-plane/VIEW-CONTRACT.md new file mode 100644 index 000000000..8f6f00abb --- /dev/null +++ b/docs/control-plane/VIEW-CONTRACT.md @@ -0,0 +1,141 @@ +# ECC control-plane live view: `ecc.control-plane.view.v1` + +Status: shipped with the control pane (`scripts/lib/control-pane/control-plane-view.js`). Read-only. Advisory only. + +The view joins three things the repo already computes separately and serves them as one JSON document shaped as tasks, lanes and events, so another control plane (the Ito ops board, a Hermes or Codex reader, a hook) can consume it without knowing ECC internals. + +| Input | Where it comes from | +|---|---| +| Sessions | `scripts/lib/control-pane/state.js`, the ECC2 `sessions` table | +| Pairwise proximity | `scripts/lib/agent-proximity/` (noisy-OR over `x_tree`, `x_overlap`, `x_dep`) via `scripts/lib/control-pane/proximity.js` | +| 2D projection | `scripts/lib/agent-proximity/projection.js` (rolling z-score, tails clipped at 2.5 / 97.5, PCA) | +| Coordination inventory | `scripts/lib/coordination-inventory.js` (PR #3028): declared tasks and sessions, heartbeat freshness, lease conflicts | + +## Endpoints + +Served by `node scripts/control-pane.js` (loopback only, same Host and Origin gate as the rest of the pane): + +| Route | Returns | +|---|---| +| `GET /control-plane` | Self-contained HTML page: 2D projection canvas, lanes and tasks, event feed. No external scripts. | +| `GET /api/control-plane` | The full view document below. | +| `GET /api/control-plane/events` | `{ schemaVersion, generatedAt, thresholds, events, counts }` only, for hooks and pollers. | + +The server keeps one projection window per process. Both API routes share a snapshot cached for five seconds, and concurrent refresh requests are coalesced. Reads within that interval do not add samples. After expiry, the next read refreshes the snapshot once; idle intervals do not generate synthetic samples. Failed refreshes return errors rather than healthy empty data. The page rejects failed HTTP responses and invalid view envelopes and shows `offline`. Options on `createControlPaneServer`: `projection` (`windowSize`, `clipPercentiles`), `viewOptions` (`thresholds`, `manifest`, `channelWeights`, `minWindowForZscore`), `proximityOptions` (passed to the scan). + +## Document + +```json +{ + "schemaVersion": "ecc.control-plane.view.v1", + "generatedAt": "2026-09-11T20:01:00.000Z", + "source": { "snapshotSchema": "ecc.control-pane.snapshot.v1", "repoRoot": "...", "dbPath": "..." }, + "thresholds": { "ta": 0.35, "ra": 0.7, "source": "static" }, + "lanes": [ { "id": "harness:codex", "label": "codex", "kind": "harness", "taskIds": ["session-a"] } ], + "tasks": [ { "...": "see Task" } ], + "pairs": [ { "...": "see Pair" } ], + "events": [ { "...": "see Event" } ], + "projection": { "...": "see Projection" }, + "inventory": { "...": "see Inventory" }, + "counts": { "lanes": 1, "tasks": 1, "agents": 1, "pairs": 0, "events": 0, "advisories": 0, "resolutions": 0 }, + "limits": [ "..." ] +} +``` + +### Task + +One task per session. A session with no changed files is still a task; it has no projection point and no pairs. + +| Field | Meaning | +|---|---| +| `id` | Session id, unchanged. | +| `lane` | Lane id this task belongs to. | +| `label` | Session task text, or the id. | +| `harness`, `agentType`, `state`, `pid` | From the session row. | +| `worktree` | `{ path, branch, base }` or `null`. | +| `heartbeatAt`, `updatedAt` | ISO timestamps or `null`. | +| `workingSet` | `{ fileCount, files }`: the worktree diff against its base. | +| `projection` | `{ point, pairs, maxRisk }` where `point` is `[x, y]` or `null`. `point` is the risk-weighted centroid of the task's pair points in PCA space. | +| `inventory` | `{ id, heartbeat, process, authority: "declared-only" }`. `id` is the sanitized identifier used in the inventory manifest; `heartbeat` and `process` are the #3028 observations. | + +### Lane + +A grouping of tasks. Precedence: `task-group` (session `task_group`), then `project`, then `harness`. Ids are prefixed (`group:`, `project:`, `harness:`) so a consumer can tell the kinds apart without reading `kind`. + +### Pair + +One row per agent pair from the airspace scan (only sessions with edits participate). + +| Field | Meaning | +|---|---| +| `a`, `b` | Session ids. | +| `risk`, `level` | Noisy-OR risk and the scan's level (`clear`, `advisory`, `resolution`) at the scan's thresholds. | +| `channels` | Raw `{ x_tree, x_overlap, x_dep }` in [0, 1]. | +| `normalized` | The same after z-score, clip and map-back, or equal to `channels` while the window is cold. | +| `point` | `[pc1, pc2]` PCA scores. | + +### Event + +Something an operator or a hook may act on. Ids are deterministic across polls so a consumer can dedupe. + +```json +{ + "id": "proximity.advisory:session-a|session-b:resolution", + "kind": "proximity.advisory", + "level": "resolution", + "severity": "critical", + "at": "2026-09-11T20:01:00.000Z", + "subject": { "a": "session-a", "b": "session-b", "aLabel": "...", "bLabel": "..." }, + "risk": 1, + "distance": 0, + "channels": { "x_tree": 1, "x_overlap": 1, "x_dep": 0 }, + "threshold": { "ta": 0.35, "ra": 0.7, "crossed": "ra", "source": "static" }, + "action": { "type": "steer", "steer": "session-b", "hold": "session-a" }, + "message": "Resolution advisory: session-b steers, session-a holds (risk 100%, static threshold 0.7)." +} +``` + +| Kind | Levels | Action types | Source | +|---|---|---|---| +| `proximity.advisory` | `traffic` (risk at or above `ta`), `resolution` (at or above `ra`) | `transmit` (both agents share intent), `steer` (`steer` moves, `hold` keeps course) | Every pair link, evaluated against the view's thresholds. Right-of-way: more progress, then earlier start, then stable id. | +| `inventory.lease-conflict` | `conflict` | `review` | #3028 `leaseConflicts`. Declared-only, never a lock. | + +Thresholds are static per view (`source: "static"`). A learned threshold, closure-rate escalation, and the `pause` and `wait` maneuvers are slice (b), see `TCAS-HOOK.md`. + +### Projection + +```json +{ + "method": "pca", + "channels": ["x_tree", "x_overlap", "x_dep"], + "weights": { "x_tree": 0.25, "x_overlap": 1, "x_dep": 0.9 }, + "normalization": "zscore-clipped", + "window": { "samples": 12, "percentiles": [2.5, 97.5], "channels": [ { "channel": "x_tree", "mean": 0.39, "stddev": 0.42, "clipLow": -0.92, "clipHigh": 1.45 } ] }, + "pca": { "loadings": [ { "x_tree": 0.12, "x_overlap": 0.87, "x_dep": -0.47 }, { "...": "..." } ], "explainedVariance": [0.6, 0.39] }, + "agents": [ { "agentId": "session-a", "point": [0.18, 0.41], "pairs": 3, "maxRisk": 1 } ] +} +``` + +Pipeline per poll: every pair's channel vector is pushed into a rolling window (default 512 samples). Once the window holds at least 8 samples, each channel is z-scored against the window, clipped to the window's 2.5th and 97.5th percentile (in z units), mapped back to [0, 1], multiplied by the static channel weight, and the weighted matrix goes through PCA (Jacobi on the 3x3 covariance). Below 8 samples the raw channel values are used and `normalization` says `raw`. A channel with zero variance maps to 0.5. Degenerate inputs (fewer than two pairs, zero total variance) give zero scores, never NaN. + +The projection is a display. It never changes `risk`, the advisory level, or right-of-way. + +### Inventory + +The #3028 report with the per-task rows folded into `tasks[].inventory`. Kept at the top level: `status` (`ok` or `unavailable` with `reason`), `truncated` (more than 64 sessions), `observedAt`, `mode: "read-only"`, `activity`, `leaseConflicts`, `warnings`, `coverage`, `limits`. The manifest is built from the live sessions (ids sanitized to the inventory alphabet, paths from the working set, heartbeat from the session row, declared session status `open` for running/pending/idle, `closed` for completed/failed/stopped). An external manifest (`viewOptions.manifest`) can add `goals`, `leases`, `repositories` and extra `tasks`; the inventory then reports lease conflicts and goal activity for them. + +## Reuse in the Ito ops control plane + +The shape to copy is `task`, `lane`, `event`: + +- a **task** has an `id`, a `lane`, a `state`, an optional position, and an observation block whose `authority` says how much to trust it; +- a **lane** is a named group with ordered `taskIds`; +- an **event** has a stable `id`, a `kind`, a `level`, a `severity`, an `at`, a `subject`, an `action` with a `type`, and a human `message`. + +Nothing in the shape is ECC-specific except the event kinds. An ops board that renders lanes of tasks and a feed of events can render this document as-is, and can emit its own kinds (`deal.stalled`, `bridge.down`) into the same feed. + +## What this does not do + +- No leases are acquired, no agent is paused or steered. Consumers act; the view reports. +- No conflict-reduction percentage is claimed. The 85 percent goal in the push plan is measured two weeks before and after slice (b), not here. +- No semantic, call-graph or frequency channel yet (slice (g)). PCA picks new channels up automatically when they land in the scan. diff --git a/scripts/lib/agent-proximity/distance.js b/scripts/lib/agent-proximity/distance.js index 2cddcbb89..8042d3e5e 100644 --- a/scripts/lib/agent-proximity/distance.js +++ b/scripts/lib/agent-proximity/distance.js @@ -270,6 +270,21 @@ function agentPriority(agent) { return { progress, ageMs: startedAt ? Date.now() - startedAt : 0 }; } +/** + * Right-of-way between two agents: more progress wins; tie goes to the earlier + * start (greater age); final deterministic tiebreak on agentId so the maneuver + * is coordinated. Returns { hold, steer } as agentIds. + */ +function rightOfWay(a, b) { + const pa = agentPriority(a); + const pb = agentPriority(b); + let aHasPriority; + if (pa.progress !== pb.progress) aHasPriority = pa.progress > pb.progress; + else if (pa.ageMs !== pb.ageMs) aHasPriority = pa.ageMs > pb.ageMs; + else aHasPriority = String(a.agentId) < String(b.agentId); + return { hold: aHasPriority ? a.agentId : b.agentId, steer: aHasPriority ? b.agentId : a.agentId }; +} + /** * TCAS-style advisory between two agents given their collision risk. * Returns { level: 'clear'|'advisory'|'resolution', risk, transmit, steer, hold }. @@ -284,17 +299,7 @@ function advise(a, b, graph = {}, options = {}) { return { level: 'clear', risk, distance, channels, transmit: false, steer: null, hold: null }; } - const pa = agentPriority(a); - const pb = agentPriority(b); - // Right-of-way: more progress wins; tie → earlier start (greater age) wins; - // final deterministic tiebreak on agentId so the maneuver is coordinated. - let aHasPriority; - if (pa.progress !== pb.progress) aHasPriority = pa.progress > pb.progress; - else if (pa.ageMs !== pb.ageMs) aHasPriority = pa.ageMs > pb.ageMs; - else aHasPriority = String(a.agentId) < String(b.agentId); - - const hold = aHasPriority ? a.agentId : b.agentId; - const steer = aHasPriority ? b.agentId : a.agentId; + const { hold, steer } = rightOfWay(a, b); if (risk < thresholds.ra) { // Traffic advisory: exchange intent, no one has to move yet. @@ -324,6 +329,7 @@ module.exports = { treeRisk, collisionRisk, agentPriority, + rightOfWay, advise, closureRate, _internal: { normalizePath, segments, jaccard } diff --git a/scripts/lib/agent-proximity/index.js b/scripts/lib/agent-proximity/index.js index 6815fe291..429c2e17d 100644 --- a/scripts/lib/agent-proximity/index.js +++ b/scripts/lib/agent-proximity/index.js @@ -135,7 +135,8 @@ function scanAirspace(agents, graph = {}, options = {}) { b: b.agentId, risk: verdict.risk, distance: verdict.distance, - level: verdict.level + level: verdict.level, + channels: verdict.channels }); if (verdict.level !== 'clear') { advisories.push({ a: a.agentId, b: b.agentId, ...verdict }); diff --git a/scripts/lib/agent-proximity/projection.js b/scripts/lib/agent-proximity/projection.js new file mode 100644 index 000000000..08a62a685 --- /dev/null +++ b/scripts/lib/agent-proximity/projection.js @@ -0,0 +1,305 @@ +'use strict'; + +/** + * 2D projection of the pairwise proximity channels for the control-plane view. + * + * Input: one row per agent pair, the shipped channel vector + * x = [x_tree, x_overlap, x_dep] each in [0, 1] + * (distance.js: treeRisk, overlapRisk, dependencyRisk). + * + * Pipeline (COMPETITION-AND-VISION section 4, "Normalization and projection"): + * 1. z-score each channel against a rolling window of pair samples, + * 2. clip the tails at the 2.5th and 97.5th percentile of that window, + * 3. map back to [0, 1], + * 4. apply the static channel weights (same omega as the noisy-OR), + * 5. PCA over the weighted matrix, keep the first two components. + * + * Agent positions are the risk-weighted centroid of the projected points of + * the pairs the agent belongs to. Nothing here changes the risk or the + * advisory: the projection is a display, not a decision. + * + * No runtime dependencies. The eigen-decomposition is a Jacobi sweep over the + * 3x3 covariance matrix, which is exact enough for a display. + */ + +const CHANNEL_ORDER = ['tree', 'overlap', 'dependency']; +const CHANNEL_LABELS = { tree: 'x_tree', overlap: 'x_overlap', dependency: 'x_dep' }; + +const PROJECTION_DEFAULTS = { + windowSize: 512, + minWindowForZscore: 8, + clipPercentiles: [2.5, 97.5], + components: 2 +}; + +function finite(x) { + return Number.isFinite(x) ? x : 0; +} + +function mean(values) { + if (values.length === 0) return 0; + let s = 0; + for (const v of values) s += v; + return s / values.length; +} + +function stddev(values, mu) { + if (values.length < 2) return 0; + let s = 0; + for (const v of values) s += (v - mu) * (v - mu); + return Math.sqrt(s / (values.length - 1)); +} + +/** + * Linear-interpolated percentile (p in [0, 100]) of a numeric array. + */ +function percentile(values, p) { + const sorted = values.filter(Number.isFinite).slice().sort((a, b) => a - b); + if (sorted.length === 0) return 0; + if (sorted.length === 1) return sorted[0]; + const rank = (Math.min(100, Math.max(0, p)) / 100) * (sorted.length - 1); + const lo = Math.floor(rank); + const hi = Math.ceil(rank); + if (lo === hi) return sorted[lo]; + return sorted[lo] + (sorted[hi] - sorted[lo]) * (rank - lo); +} + +/** + * Rolling window of pair channel samples. Each push records one sample vector; + * the window keeps the newest `size` samples. `stats()` returns, per channel, + * the mean, standard deviation and clip bounds (in z units) used to normalize. + */ +function createProjectionWindow(options = {}) { + const size = Number.isFinite(options.windowSize) && options.windowSize > 0 ? Math.floor(options.windowSize) : PROJECTION_DEFAULTS.windowSize; + const [pLo, pHi] = Array.isArray(options.clipPercentiles) && options.clipPercentiles.length === 2 ? options.clipPercentiles : PROJECTION_DEFAULTS.clipPercentiles; + const samples = []; + + return { + size, + push(vector) { + const row = CHANNEL_ORDER.map((_, i) => finite(vector[i])); + samples.push(row); + if (samples.length > size) samples.splice(0, samples.length - size); + return samples.length; + }, + get length() { + return samples.length; + }, + stats() { + const per = CHANNEL_ORDER.map((channel, i) => { + const column = samples.map(row => row[i]); + const mu = mean(column); + const sigma = stddev(column, mu); + const z = sigma > 0 ? column.map(v => (v - mu) / sigma) : column.map(() => 0); + return { + channel, + mean: mu, + stddev: sigma, + clipLow: percentile(z, pLo), + clipHigh: percentile(z, pHi) + }; + }); + return { samples: samples.length, percentiles: [pLo, pHi], channels: per }; + }, + reset() { + samples.length = 0; + } + }; +} + +/** + * z-score one sample against the window stats, clip to the percentile bounds, + * map back to [0, 1]. A channel with zero variance maps to 0.5. + */ +function normalizeSample(vector, stats) { + return CHANNEL_ORDER.map((_, i) => { + const s = stats.channels[i]; + const v = finite(vector[i]); + if (!(s.stddev > 0)) return 0.5; + const z = (v - s.mean) / s.stddev; + const lo = s.clipLow; + const hi = s.clipHigh; + if (!(hi > lo)) return 0.5; + const clipped = Math.min(hi, Math.max(lo, z)); + return (clipped - lo) / (hi - lo); + }); +} + +/** + * Jacobi eigen-decomposition of a small symmetric matrix. Returns eigenvalues + * (descending) and the matching unit eigenvectors (as columns). + */ +function symmetricEigen(matrix) { + const n = matrix.length; + const a = matrix.map(row => row.slice()); + const v = Array.from({ length: n }, (_, i) => Array.from({ length: n }, (_, j) => (i === j ? 1 : 0))); + for (let sweep = 0; sweep < 64; sweep += 1) { + let off = 0; + for (let p = 0; p < n; p += 1) for (let q = p + 1; q < n; q += 1) off += a[p][q] * a[p][q]; + if (off < 1e-18) break; + for (let p = 0; p < n; p += 1) { + for (let q = p + 1; q < n; q += 1) { + if (Math.abs(a[p][q]) < 1e-14) continue; + const theta = (a[q][q] - a[p][p]) / (2 * a[p][q]); + const t = Math.sign(theta || 1) / (Math.abs(theta) + Math.sqrt(theta * theta + 1)); + const c = 1 / Math.sqrt(t * t + 1); + const s = t * c; + for (let k = 0; k < n; k += 1) { + const akp = a[k][p]; + const akq = a[k][q]; + a[k][p] = c * akp - s * akq; + a[k][q] = s * akp + c * akq; + } + for (let k = 0; k < n; k += 1) { + const apk = a[p][k]; + const aqk = a[q][k]; + a[p][k] = c * apk - s * aqk; + a[q][k] = s * apk + c * aqk; + } + for (let k = 0; k < n; k += 1) { + const vkp = v[k][p]; + const vkq = v[k][q]; + v[k][p] = c * vkp - s * vkq; + v[k][q] = s * vkp + c * vkq; + } + } + } + } + const order = Array.from({ length: n }, (_, i) => i).sort((i, j) => a[j][j] - a[i][i]); + return { + values: order.map(i => a[i][i]), + vectors: order.map(i => v.map(row => row[i])) + }; +} + +/** + * PCA over a row matrix. Returns the scores for the first `components` + * components, the loadings (unit eigenvectors) and the explained variance. + * Fewer than two rows, or zero total variance, yields all-zero scores. + */ +function pca(rows, components = PROJECTION_DEFAULTS.components) { + const n = rows.length; + const dims = n > 0 ? rows[0].length : CHANNEL_ORDER.length; + const k = Math.max(1, Math.min(components, dims)); + const centre = Array.from({ length: dims }, (_, d) => mean(rows.map(r => r[d]))); + const zeroScores = rows.map(() => new Array(k).fill(0)); + if (n < 2) { + return { scores: zeroScores, loadings: [], explainedVariance: new Array(k).fill(0), centre }; + } + const cov = Array.from({ length: dims }, () => new Array(dims).fill(0)); + for (const row of rows) { + for (let i = 0; i < dims; i += 1) { + for (let j = i; j < dims; j += 1) { + cov[i][j] += (row[i] - centre[i]) * (row[j] - centre[j]); + } + } + } + for (let i = 0; i < dims; i += 1) for (let j = i; j < dims; j += 1) { + cov[i][j] /= n - 1; + cov[j][i] = cov[i][j]; + } + const total = cov.reduce((s, row, i) => s + row[i], 0); + if (!(total > 1e-12)) { + return { scores: zeroScores, loadings: [], explainedVariance: new Array(k).fill(0), centre }; + } + const eig = symmetricEigen(cov); + const loadings = eig.vectors.slice(0, k); + const scores = rows.map(row => loadings.map(vec => vec.reduce((s, w, d) => s + w * (row[d] - centre[d]), 0))); + const explainedVariance = eig.values.slice(0, k).map(val => Math.max(0, val) / total); + return { scores, loadings, explainedVariance, centre }; +} + +function channelVector(channels) { + return CHANNEL_ORDER.map(key => finite(channels && channels[key])); +} + +/** + * Project a set of pair links ({ a, b, risk, channels }) to 2D. + * + * The window is optional; when given, each link's channel vector is pushed + * into it and the normalization uses the window stats (rolling z-score plus + * tail clip). Without a window, or while the window holds fewer than + * `minWindowForZscore` samples, the raw [0, 1] channel values are used and the + * result says so (`normalization: 'raw'`). + * + * @returns {{ pairs, agents, normalization, window, pca }} + */ +function projectPairs(links, options = {}) { + const list = Array.isArray(links) ? links.filter(l => l && l.a !== undefined && l.b !== undefined) : []; + const weights = { tree: 0.25, overlap: 1.0, dependency: 0.9, ...(options.channelWeights || {}) }; + const window = options.window || null; + const minWindow = Number.isFinite(options.minWindowForZscore) ? options.minWindowForZscore : PROJECTION_DEFAULTS.minWindowForZscore; + + const raw = list.map(l => channelVector(l.channels)); + if (window && options.sample !== false) for (const vec of raw) window.push(vec); + + let stats = null; + let normalization = 'raw'; + let normalized = raw; + if (window && window.length >= minWindow) { + stats = window.stats(); + normalized = raw.map(vec => normalizeSample(vec, stats)); + normalization = 'zscore-clipped'; + } + const weighted = normalized.map(vec => vec.map((v, i) => v * finite(weights[CHANNEL_ORDER[i]]))); + const result = pca(weighted, options.components || PROJECTION_DEFAULTS.components); + + const pairs = list.map((l, i) => ({ + a: l.a, + b: l.b, + risk: finite(l.risk), + level: l.level || null, + channels: Object.fromEntries(CHANNEL_ORDER.map((key, d) => [CHANNEL_LABELS[key], raw[i][d]])), + normalized: Object.fromEntries(CHANNEL_ORDER.map((key, d) => [CHANNEL_LABELS[key], normalized[i][d]])), + point: result.scores[i] + })); + + // Agent position: risk-weighted centroid of its pair points. A floor keeps + // a clear pair from vanishing, so every agent with a pair gets a position. + const byAgent = new Map(); + for (const pair of pairs) { + const w = 0.05 + pair.risk; + for (const id of [pair.a, pair.b]) { + const acc = byAgent.get(id) || { sum: pair.point.map(() => 0), w: 0, pairs: 0, maxRisk: 0 }; + pair.point.forEach((x, d) => { + acc.sum[d] += x * w; + }); + acc.w += w; + acc.pairs += 1; + acc.maxRisk = Math.max(acc.maxRisk, pair.risk); + byAgent.set(id, acc); + } + } + const agents = [...byAgent.entries()].map(([agentId, acc]) => ({ + agentId, + point: acc.sum.map(x => (acc.w > 0 ? x / acc.w : 0)), + pairs: acc.pairs, + maxRisk: acc.maxRisk + })); + + return { + method: 'pca', + channels: CHANNEL_ORDER.map(key => CHANNEL_LABELS[key]), + weights: Object.fromEntries(CHANNEL_ORDER.map(key => [CHANNEL_LABELS[key], finite(weights[key])])), + normalization, + window: stats ? { samples: stats.samples, percentiles: stats.percentiles, channels: stats.channels.map(c => ({ ...c, channel: CHANNEL_LABELS[c.channel] })) } : { samples: window ? window.length : 0, percentiles: PROJECTION_DEFAULTS.clipPercentiles, channels: [] }, + pca: { + loadings: result.loadings.map(vec => Object.fromEntries(CHANNEL_ORDER.map((key, d) => [CHANNEL_LABELS[key], vec[d]]))), + explainedVariance: result.explainedVariance + }, + pairs, + agents + }; +} + +module.exports = { + PROJECTION_DEFAULTS, + CHANNEL_ORDER, + CHANNEL_LABELS, + percentile, + createProjectionWindow, + normalizeSample, + pca, + projectPairs, + _internal: { symmetricEigen, mean, stddev } +}; diff --git a/scripts/lib/control-pane/control-plane-view-ui.js b/scripts/lib/control-pane/control-plane-view-ui.js new file mode 100644 index 000000000..2fe9e95cd --- /dev/null +++ b/scripts/lib/control-pane/control-plane-view-ui.js @@ -0,0 +1,243 @@ +'use strict'; + +/** + * Self-contained control-plane live view page, served at /control-plane. + * + * Draws the 2D PCA projection of the agent pairs (projection.js) on a canvas, + * the lanes and tasks beside it, and the advisory event feed. Polls + * /api/control-plane. No external scripts, no framework: it has to work on a + * loopback server with a strict CSP and offline. + */ + +function renderControlPlaneViewHtml() { + return ` + + + + +ECC Control Plane + + + +
+

ECC Control Plane

+ connecting... + +
+
+
+ +
+
+
clear
+
traffic advisory (transmit)
+
resolution advisory (steer)
+
+
+
+

Events

+
No events.
+

Lanes

+
No tasks.
+
+
+ + +`; +} + +module.exports = { renderControlPlaneViewHtml }; diff --git a/scripts/lib/control-pane/control-plane-view.js b/scripts/lib/control-pane/control-plane-view.js new file mode 100644 index 000000000..32f6a57ee --- /dev/null +++ b/scripts/lib/control-pane/control-plane-view.js @@ -0,0 +1,358 @@ +'use strict'; + +/** + * ECC control-plane live view. + * + * One JSON document, `ecc.control-plane.view.v1`, that joins three things the + * repo already computes separately: + * + * 1. the control-pane session snapshot (state.js): who is running where, + * 2. the agent-proximity airspace scan (agent-proximity + proximity.js): + * pairwise collision risk over the shipped channels x_tree, x_overlap, + * x_dep, with the 2D PCA projection from agent-proximity/projection.js, + * 3. the coordination inventory (coordination-inventory.js, PR #3028): + * declared tasks and sessions, heartbeat freshness, lease conflicts. + * + * The output is shaped as tasks, lanes and events so another control plane + * (the Ito ops board) can consume it without knowing ECC internals: + * + * task = one agent session (id, lane, harness, state, worktree, working + * set size, projected point, inventory observation) + * lane = a grouping of tasks (task group, project, or harness) + * event = something an operator or a hook may act on. Today: a proximity + * advisory at a static threshold, or a lease conflict. + * + * Everything here is read-only and advisory. The view does not acquire + * leases, does not steer agents and does not claim a conflict-reduction + * number. See docs/control-plane/VIEW-CONTRACT.md. + */ + +const { DEFAULTS, rightOfWay } = require('../agent-proximity/distance'); +const { projectPairs, createProjectionWindow } = require('../agent-proximity/projection'); + +const VIEW_SCHEMA_VERSION = 'ecc.control-plane.view.v1'; +const EVENT_KINDS = { + advisory: 'proximity.advisory', + leaseConflict: 'inventory.lease-conflict' +}; + +const IDENTIFIER = /^[a-zA-Z0-9][a-zA-Z0-9_.:-]*$/; +const OPEN_STATES = new Set(['running', 'pending', 'idle']); +const CLOSED_STATES = new Set(['completed', 'failed', 'stopped']); + +function isoOrNull(value) { + if (!value) return null; + const ms = Date.parse(value); + return Number.isFinite(ms) ? new Date(ms).toISOString() : null; +} + +/** + * Map a session id to an identifier the inventory accepts. Replaces anything + * outside the allowed alphabet, strips a leading non-alphanumeric run, and + * falls back to a positional id. Callers get the mapping back so a consumer + * can join inventory rows to tasks. + */ +function inventoryIdFor(id, index, taken) { + let candidate = String(id || '') + .replace(/[^a-zA-Z0-9_.:-]/g, '-') + .replace(/^[^a-zA-Z0-9]+/, '') + .slice(0, 200); + if (!candidate || ['__proto__', 'constructor', 'prototype'].includes(candidate)) candidate = `task-${index + 1}`; + let unique = candidate; + let n = 2; + while (taken.has(unique)) { + unique = `${candidate.slice(0, 190)}-${n}`; + n += 1; + } + taken.add(unique); + return IDENTIFIER.test(unique) ? unique : `task-${index + 1}`; +} + +function laneFor(session) { + if (session.taskGroup) return { id: `group:${session.taskGroup}`, label: session.taskGroup, kind: 'task-group' }; + if (session.project) return { id: `project:${session.project}`, label: session.project, kind: 'project' }; + const harness = session.harness || 'unknown'; + return { id: `harness:${harness}`, label: harness, kind: 'harness' }; +} + +function sessionDeclarationStatus(state) { + if (OPEN_STATES.has(state)) return 'open'; + if (CLOSED_STATES.has(state)) return 'closed'; + return 'unknown'; +} + +/** + * Build the #3028 manifest from live sessions plus the working sets the + * proximity scan already extracted. Declared-only by construction: the + * inventory library labels every row `declared-only` and this view keeps + * that label. + */ +function buildInventoryManifest(sessions, agentsById, options = {}) { + const taken = new Set(); + const idMap = new Map(); + const tasks = []; + const declaredSessions = []; + const limited = (sessions || []).slice(0, 64); + limited.forEach((session, index) => { + const invId = inventoryIdFor(session.id, index, taken); + idMap.set(session.id, invId); + const agent = agentsById.get(session.id); + const paths = (agent ? agent.files : []).filter(p => typeof p === 'string' && !p.startsWith('/') && !/^[A-Za-z]:/.test(p) && !p.split('/').some(x => !x || x === '.' || x === '..')).slice(0, 128); + tasks.push({ + id: invId, + repoId: null, + paths, + pid: Number.isSafeInteger(session.pid) && session.pid > 0 ? session.pid : null, + status: String(session.state || 'unknown').slice(0, 200) || 'unknown', + heartbeatAt: isoOrNull(session.lastHeartbeatAt), + statusFileModifiedAt: isoOrNull(session.updatedAt) + }); + declaredSessions.push({ + id: invId, + taskId: invId, + goalId: null, + status: sessionDeclarationStatus(session.state), + updatedAt: isoOrNull(session.lastHeartbeatAt || session.updatedAt) + }); + }); + const extra = options.manifest && typeof options.manifest === 'object' ? options.manifest : {}; + return { + manifest: { + version: 1, + repositories: Array.isArray(extra.repositories) ? extra.repositories : [], + tasks: [...tasks, ...(Array.isArray(extra.tasks) ? extra.tasks : [])], + sessions: [...declaredSessions, ...(Array.isArray(extra.sessions) ? extra.sessions : [])], + goals: Array.isArray(extra.goals) ? extra.goals : [], + leases: Array.isArray(extra.leases) ? extra.leases : [] + }, + idMap, + truncated: (sessions || []).length > limited.length + }; +} + +function runInventory(sessions, agentsById, options = {}) { + const built = buildInventoryManifest(sessions, agentsById, options); + try { + const { buildInventory } = options.inventoryModule || require('../coordination-inventory'); + const report = buildInventory(built.manifest, { now: options.now, resources: options.resources }); + return { status: 'ok', idMap: built.idMap, truncated: built.truncated, report }; + } catch (error) { + return { status: 'unavailable', idMap: built.idMap, truncated: built.truncated, reason: error.message, report: null }; + } +} + +/** + * Agent shape the right-of-way rule needs, rebuilt from the proximity + * snapshot's agent summaries (progress = recency-weighted file count). + */ +function priorityAgent(summary, agentId) { + if (!summary) return { agentId, files: [], startedAt: null }; + const progress = Number.isFinite(summary.progress) ? summary.progress : summary.fileCount || 0; + return { agentId, startedAt: summary.startedAt || null, files: [{ path: '', weight: progress }] }; +} + +/** + * Static-threshold advisory events, derived from every pair link against the + * view's own thresholds so an override changes the events, not only labels. + * The risk itself comes from the scan (noisy-OR, unchanged). + */ +function advisoryEvents(links, agentsById, thresholds, at) { + const events = []; + for (const link of links || []) { + if (!link || !Number.isFinite(link.risk) || link.risk < thresholds.ta) continue; + const resolution = link.risk >= thresholds.ra; + const level = resolution ? 'resolution' : 'traffic'; + const a = agentsById.get(link.a); + const b = agentsById.get(link.b); + const aLabel = (a && a.label) || link.a; + const bLabel = (b && b.label) || link.b; + const way = resolution ? rightOfWay(priorityAgent(a, link.a), priorityAgent(b, link.b)) : { steer: null, hold: null }; + const channels = link.channels || {}; + events.push({ + id: `${EVENT_KINDS.advisory}:${link.a}|${link.b}:${level}`, + kind: EVENT_KINDS.advisory, + level, + severity: resolution ? 'critical' : 'warning', + at, + subject: { a: link.a, b: link.b, aLabel, bLabel }, + risk: link.risk, + distance: Number.isFinite(link.distance) ? link.distance : 1 - link.risk, + channels: { + x_tree: Number.isFinite(channels.tree) ? channels.tree : null, + x_overlap: Number.isFinite(channels.overlap) ? channels.overlap : null, + x_dep: Number.isFinite(channels.dependency) ? channels.dependency : null + }, + threshold: { ta: thresholds.ta, ra: thresholds.ra, crossed: resolution ? 'ra' : 'ta', source: 'static' }, + action: resolution ? { type: 'steer', steer: way.steer, hold: way.hold } : { type: 'transmit', steer: null, hold: null }, + message: resolution + ? `Resolution advisory: ${way.steer} steers, ${way.hold} holds (risk ${Math.round(link.risk * 100)}%, static threshold ${thresholds.ra}).` + : `Traffic advisory: ${link.a} and ${link.b} transmit intent (risk ${Math.round(link.risk * 100)}%, static threshold ${thresholds.ta}).` + }); + } + events.sort((x, y) => y.risk - x.risk); + return events; +} + +function leaseConflictEvents(report, at) { + if (!report || !Array.isArray(report.leaseConflicts)) return []; + return report.leaseConflicts.map(conflict => ({ + id: `${EVENT_KINDS.leaseConflict}:${conflict.resource}`, + kind: EVENT_KINDS.leaseConflict, + level: 'conflict', + severity: 'warning', + at, + subject: { resource: conflict.resource, owners: conflict.owners }, + action: { type: 'review', steer: null, hold: null }, + message: `Declared lease conflict on ${conflict.resource}: ${conflict.owners.join(', ')}. Declared-only, not a lock.` + })); +} + +/** + * Build the live view from a control-pane snapshot that already carries a + * `proximity` field (buildControlPaneSnapshot with includeProximity: true). + * + * @param {object} snapshot control-pane snapshot + * @param {object} [options] { window, thresholds, now, manifest, resources, channelWeights } + */ +function buildControlPlaneView(snapshot, options = {}) { + const at = options.now || new Date().toISOString(); + const thresholds = { ...DEFAULTS.thresholds, ...(options.thresholds || {}) }; + const sessions = Array.isArray(snapshot && snapshot.sessions) ? snapshot.sessions : []; + const prox = (snapshot && snapshot.proximity) || {}; + const agents = Array.isArray(prox.agents) ? prox.agents : []; + const agentsById = new Map(agents.map(a => [a.agentId, a])); + + const projection = projectPairs(prox.links || [], { + window: options.window, + channelWeights: options.channelWeights, + sample: options.sample, + minWindowForZscore: options.minWindowForZscore + }); + const pointByAgent = new Map(projection.agents.map(a => [a.agentId, a])); + + const inventory = runInventory(sessions, agentsById, { + now: at, + manifest: options.manifest, + resources: options.resources, + inventoryModule: options.inventoryModule + }); + const inventoryTaskById = new Map(); + if (inventory.report) for (const task of inventory.report.tasks || []) inventoryTaskById.set(task.id, task); + + const lanes = new Map(); + const tasks = sessions.map(session => { + const lane = laneFor(session); + if (!lanes.has(lane.id)) lanes.set(lane.id, { ...lane, taskIds: [] }); + lanes.get(lane.id).taskIds.push(session.id); + const agent = agentsById.get(session.id); + const projected = pointByAgent.get(session.id); + const invId = inventory.idMap.get(session.id) || null; + const invTask = invId ? inventoryTaskById.get(invId) : null; + return { + id: session.id, + lane: lane.id, + label: session.task || session.id, + harness: session.harness || 'unknown', + agentType: session.agentType || '', + state: session.state || 'unknown', + pid: session.pid === undefined ? null : session.pid, + worktree: session.worktree || null, + heartbeatAt: isoOrNull(session.lastHeartbeatAt), + updatedAt: isoOrNull(session.updatedAt), + workingSet: { fileCount: agent ? agent.fileCount : 0, files: agent ? agent.files : [] }, + projection: projected ? { point: projected.point, pairs: projected.pairs, maxRisk: projected.maxRisk } : { point: null, pairs: 0, maxRisk: 0 }, + inventory: invTask ? { id: invId, heartbeat: invTask.heartbeat, process: invTask.process, authority: 'declared-only' } : { id: invId, heartbeat: null, process: null, authority: 'declared-only' } + }; + }); + + const events = [...advisoryEvents(prox.links, agentsById, thresholds, at), ...leaseConflictEvents(inventory.report, at)]; + + const { pairs, agents: projectedAgents, ...projectionMeta } = projection; + return { + schemaVersion: VIEW_SCHEMA_VERSION, + generatedAt: at, + source: { + snapshotSchema: snapshot ? snapshot.schemaVersion || null : null, + repoRoot: snapshot ? snapshot.repoRoot || null : null, + dbPath: snapshot ? snapshot.dbPath || null : null + }, + thresholds: { ta: thresholds.ta, ra: thresholds.ra, source: 'static' }, + lanes: [...lanes.values()], + tasks, + pairs, + events, + projection: { ...projectionMeta, agents: projectedAgents }, + inventory: inventory.report + ? { + status: 'ok', + truncated: inventory.truncated, + observedAt: inventory.report.observedAt, + mode: inventory.report.mode, + activity: inventory.report.activity, + leaseConflicts: inventory.report.leaseConflicts, + warnings: inventory.report.warnings, + coverage: inventory.report.coverage, + limits: inventory.report.limits + } + : { status: inventory.status, truncated: inventory.truncated, reason: inventory.reason || null }, + counts: { + lanes: lanes.size, + tasks: tasks.length, + agents: agents.length, + pairs: pairs.length, + events: events.length, + advisories: events.filter(e => e.kind === EVENT_KINDS.advisory).length, + resolutions: events.filter(e => e.kind === EVENT_KINDS.advisory && e.level === 'resolution').length + }, + limits: [ + 'Advisories use static thresholds; no learned threshold and no conflict-reduction claim.', + 'Projection is a display over the shipped channels x_tree, x_overlap, x_dep; it does not change risk.', + 'Inventory rows are declared-only observations; leases are not locks.', + 'The view does not steer, pause or lock any agent.' + ] + }; +} + +/** + * Stateful view builder for a long-lived server: keeps one projection window + * so z-scores roll over ticks. `buildSnapshot()` is injected (it is the + * control-pane snapshot with includeProximity: true). + */ +function createControlPlaneViewSource(deps = {}) { + const window = deps.window || createProjectionWindow(deps.projection || {}); + const clock = deps.clock || Date.now; + const interval = deps.sampleIntervalMs === undefined ? 5000 : deps.sampleIntervalMs; + if (!Number.isFinite(interval) || interval <= 0) throw new Error('sampleIntervalMs must be positive and finite'); + let cached = null; + let pending = null; + let expiresAt = 0; + async function refresh() { + const snapshot = await deps.buildSnapshot(); + const view = buildControlPlaneView(snapshot, { ...deps.viewOptions, window }); + cached = { snapshot, view }; + expiresAt = clock() + interval; + return cached; + } + return { + window, + async build(extra = {}) { + if (!cached || clock() >= expiresAt) { + if (!pending) pending = refresh().finally(() => { pending = null; }); + await pending; + } + if (Object.keys(extra).length === 0) return cached.view; + return buildControlPlaneView(cached.snapshot, { + ...deps.viewOptions, ...extra, now: extra.now || cached.view.generatedAt, window, sample: false + }); + } + }; +} + +module.exports = { + VIEW_SCHEMA_VERSION, + EVENT_KINDS, + buildControlPlaneView, + createControlPlaneViewSource, + buildInventoryManifest, + _internal: { inventoryIdFor, laneFor, sessionDeclarationStatus, advisoryEvents, leaseConflictEvents } +}; diff --git a/scripts/lib/control-pane/proximity-viz.js b/scripts/lib/control-pane/proximity-viz.js index 2780e5bcc..5c40a0ac4 100644 --- a/scripts/lib/control-pane/proximity-viz.js +++ b/scripts/lib/control-pane/proximity-viz.js @@ -49,6 +49,7 @@ function renderProximityVizHtml() {

ECC - Agent Airspace

connecting... + 2D control plane
diff --git a/scripts/lib/control-pane/proximity.js b/scripts/lib/control-pane/proximity.js index 7e451baf0..a958ef36f 100644 --- a/scripts/lib/control-pane/proximity.js +++ b/scripts/lib/control-pane/proximity.js @@ -122,10 +122,21 @@ function buildProximitySnapshot(sessions, options = {}) { const agents = sessionsToAgents(sessions, options); // Need at least two participating agents for a collision to be possible. + const agentSummaries = agents.map(a => ({ + agentId: a.agentId, + label: a.label, + startedAt: a.startedAt, + fileCount: a.files.length, + progress: a.files.reduce((s, f) => s + (f.weight ?? 1), 0), + files: a.files.map(f => f.path) + })); + if (agents.length < 2) { return { enabled: true, advisories: [], + triggers: [], + agents: agentSummaries, positions: agents.map(a => ({ agentId: a.agentId, position: [0, 0, 0], fileCount: a.files.length })), links: [], counts: { agents: agents.length, advisories: 0, resolutions: 0 } @@ -151,6 +162,7 @@ function buildProximitySnapshot(sessions, options = {}) { enabled: true, advisories, triggers: buildProximityTriggers(scan.advisories), + agents: agentSummaries, positions: scan.positions, links: scan.links, counts: scan.counts diff --git a/scripts/lib/control-pane/server.js b/scripts/lib/control-pane/server.js index bfe847159..8391ac796 100644 --- a/scripts/lib/control-pane/server.js +++ b/scripts/lib/control-pane/server.js @@ -9,6 +9,8 @@ const { buildControlPaneAction } = require('./actions'); const { buildControlPaneSnapshot, resolveControlPaneConfig } = require('./state'); const { renderControlPaneHtml } = require('./ui'); const { renderProximityVizHtml } = require('./proximity-viz'); +const { renderControlPlaneViewHtml } = require('./control-plane-view-ui'); +const { createControlPlaneViewSource } = require('./control-plane-view'); const { claimWorkItem, moveWorkItem } = require('./work-item-mutations'); // Run a single write against the local work-item store, then close it. Kept @@ -185,6 +187,24 @@ function createControlPaneServer(options = {}) { const baseQuery = options.query || ''; const allowedHostnames = buildAllowedHostnames(host); + // Live control-plane view: sessions + proximity scan + coordination + // inventory, joined as tasks/lanes/events with a 2D projection. The view + // source owns the rolling projection window so z-scores span ticks. + const viewSource = createControlPlaneViewSource({ + projection: options.projection || {}, + viewOptions: options.viewOptions || {}, + buildSnapshot: () => + buildControlPaneSnapshot({ + repoRoot, + dbPath: resolvedConfig.dbPath, + stateDbPath: resolvedConfig.stateDbPath, + config: resolvedConfig, + allowActions, + includeProximity: true, + proximityOptions: options.proximityOptions + }) + }); + const server = http.createServer(async (req, res) => { try { if (!isAllowedHostHeader(req.headers.host, allowedHostnames)) { @@ -257,6 +277,29 @@ function createControlPaneServer(options = {}) { return; } + // Control-plane live view: 2D projection + advisory events + inventory. + if (req.method === 'GET' && requestUrl.pathname === '/control-plane') { + sendText(res, 200, renderControlPlaneViewHtml(), 'text/html; charset=utf-8'); + return; + } + + if (req.method === 'GET' && requestUrl.pathname === '/api/control-plane') { + sendJson(res, 200, await viewSource.build()); + return; + } + + if (req.method === 'GET' && requestUrl.pathname === '/api/control-plane/events') { + const view = await viewSource.build(); + sendJson(res, 200, { + schemaVersion: view.schemaVersion, + generatedAt: view.generatedAt, + thresholds: view.thresholds, + events: view.events, + counts: { events: view.counts.events, advisories: view.counts.advisories, resolutions: view.counts.resolutions } + }); + return; + } + const actionMatch = requestUrl.pathname.match(/^\/api\/actions\/([^/]+)$/); if (req.method === 'POST' && actionMatch) { if (!allowActions) { diff --git a/tests/lib/agent-proximity-projection.test.js b/tests/lib/agent-proximity-projection.test.js new file mode 100644 index 000000000..19680b8a1 --- /dev/null +++ b/tests/lib/agent-proximity-projection.test.js @@ -0,0 +1,181 @@ +'use strict'; +/** + * Tests for scripts/lib/agent-proximity/projection.js: rolling z-score with + * tail clipping, PCA and the 2D pair/agent projection. + */ + +const assert = require('assert'); + +const { percentile, createProjectionWindow, normalizeSample, pca, projectPairs, PROJECTION_DEFAULTS, _internal } = require('../../scripts/lib/agent-proximity/projection'); +const { scanAirspace } = require('../../scripts/lib/agent-proximity'); + +let passed = 0; +let failed = 0; +function test(name, fn) { + try { + fn(); + console.log(` PASS ${name}`); + passed += 1; + } catch (e) { + console.log(` FAIL ${name}`); + console.log(` ${e.message}`); + failed += 1; + } +} + +function close(a, b, eps = 1e-6) { + return Math.abs(a - b) <= eps; +} + +console.log('\n=== Testing agent-proximity projection ===\n'); + +test('percentile: interpolates, clamps and survives empty input', () => { + assert.strictEqual(percentile([], 50), 0); + assert.strictEqual(percentile([4], 97.5), 4); + assert.strictEqual(percentile([1, 2, 3, 4, 5], 50), 3); + assert.ok(close(percentile([1, 2, 3, 4, 5], 25), 2)); + assert.strictEqual(percentile([1, 2, 3], 0), 1); + assert.strictEqual(percentile([1, 2, 3], 100), 3); + assert.strictEqual(percentile([1, 2, 3], 250), 3, 'p above 100 clamps to the max'); + assert.strictEqual(percentile([3, NaN, 1], 100), 3, 'non-finite values are ignored'); +}); + +test('window: rolls, keeps the newest samples and reports per-channel stats', () => { + const w = createProjectionWindow({ windowSize: 4 }); + for (let i = 1; i <= 6; i += 1) w.push([i, 0, i * 2]); + assert.strictEqual(w.length, 4); + const stats = w.stats(); + assert.strictEqual(stats.samples, 4); + assert.deepStrictEqual(stats.percentiles, PROJECTION_DEFAULTS.clipPercentiles); + const tree = stats.channels[0]; + assert.strictEqual(tree.channel, 'tree'); + assert.ok(close(tree.mean, 4.5), 'mean of 3,4,5,6'); + assert.ok(tree.stddev > 0); + assert.ok(tree.clipLow < 0 && tree.clipHigh > 0, 'clip bounds straddle zero in z units'); + assert.strictEqual(stats.channels[1].stddev, 0, 'constant channel has zero variance'); + w.reset(); + assert.strictEqual(w.length, 0); +}); + +test('normalizeSample: z-scores, clips the tails and maps back to [0, 1]', () => { + const w = createProjectionWindow({ windowSize: 100 }); + for (let i = 0; i < 100; i += 1) w.push([i / 100, 0.5, 0]); + const stats = w.stats(); + const low = normalizeSample([-5, 0.5, 0], stats); + const high = normalizeSample([5, 0.5, 0], stats); + const mid = normalizeSample([0.495, 0.5, 0], stats); + assert.strictEqual(low[0], 0, 'far below the 2.5th percentile clips to 0'); + assert.strictEqual(high[0], 1, 'far above the 97.5th percentile clips to 1'); + assert.ok(mid[0] > 0.4 && mid[0] < 0.6, `median lands near 0.5, got ${mid[0]}`); + assert.strictEqual(low[1], 0.5, 'zero-variance channel maps to 0.5'); + assert.strictEqual(low[2], 0.5, 'all-zero channel maps to 0.5'); + for (const v of [...low, ...high, ...mid]) assert.ok(v >= 0 && v <= 1); +}); + +test('pca: recovers the dominant axis and reports explained variance', () => { + const rows = []; + for (let i = 0; i < 40; i += 1) { + const t = i / 39; + rows.push([t, t * 0.5 + 0.001 * ((i % 3) - 1), 0.2]); + } + const out = pca(rows, 2); + assert.strictEqual(out.scores.length, rows.length); + assert.strictEqual(out.loadings.length, 2); + const first = out.loadings[0]; + const norm = Math.sqrt(first.reduce((s, x) => s + x * x, 0)); + assert.ok(close(norm, 1, 1e-6), 'loadings are unit vectors'); + assert.ok(Math.abs(first[0]) > Math.abs(first[2]), 'first component follows the varying channels, not the constant one'); + assert.ok(out.explainedVariance[0] > 0.99, `first component explains almost everything, got ${out.explainedVariance[0]}`); + assert.ok(out.explainedVariance[0] >= out.explainedVariance[1]); + const total = out.explainedVariance.reduce((s, x) => s + x, 0); + assert.ok(total <= 1 + 1e-9); +}); + +test('pca: degenerate inputs give zero scores instead of NaN', () => { + assert.deepStrictEqual(pca([], 2).scores, []); + assert.deepStrictEqual(pca([[1, 2, 3]], 2).scores, [[0, 0]]); + const flat = pca([[0.3, 0.3, 0.3], [0.3, 0.3, 0.3], [0.3, 0.3, 0.3]], 2); + assert.deepStrictEqual(flat.scores, [[0, 0], [0, 0], [0, 0]]); + assert.deepStrictEqual(flat.explainedVariance, [0, 0]); +}); + +test('symmetricEigen: diagonalizes a known 3x3 matrix', () => { + const eig = _internal.symmetricEigen([[2, 0, 0], [0, 3, 0], [0, 0, 1]]); + assert.deepStrictEqual(eig.values.map(v => Math.round(v * 1e9) / 1e9), [3, 2, 1]); + assert.ok(close(Math.abs(eig.vectors[0][1]), 1), 'top eigenvector points along the 3 axis'); +}); + +test('projectPairs: raw mode without a window, one point per pair and per agent', () => { + const links = [ + { a: 'a', b: 'b', risk: 1, level: 'resolution', channels: { tree: 1, overlap: 1, dependency: 0 } }, + { a: 'a', b: 'c', risk: 0, level: 'clear', channels: { tree: 0, overlap: 0, dependency: 0 } }, + { a: 'b', b: 'c', risk: 0.5, level: 'advisory', channels: { tree: 0.5, overlap: 0, dependency: 0.5 } } + ]; + const out = projectPairs(links); + assert.strictEqual(out.method, 'pca'); + assert.strictEqual(out.normalization, 'raw'); + assert.deepStrictEqual(out.channels, ['x_tree', 'x_overlap', 'x_dep']); + assert.deepStrictEqual(out.weights, { x_tree: 0.25, x_overlap: 1, x_dep: 0.9 }); + assert.strictEqual(out.pairs.length, 3); + assert.strictEqual(out.pairs[0].point.length, 2); + assert.deepStrictEqual(out.pairs[0].channels, { x_tree: 1, x_overlap: 1, x_dep: 0 }); + assert.deepStrictEqual(out.pairs[0].normalized, out.pairs[0].channels, 'raw mode passes channel values through'); + assert.strictEqual(out.agents.length, 3); + const a = out.agents.find(x => x.agentId === 'a'); + assert.strictEqual(a.pairs, 2); + assert.strictEqual(a.maxRisk, 1); + for (const agent of out.agents) for (const v of agent.point) assert.ok(Number.isFinite(v)); + assert.strictEqual(out.pca.loadings.length, 2); + assert.ok(out.pca.explainedVariance[0] > 0); +}); + +test('projectPairs: switches to z-score mode once the window is warm and keeps values in [0, 1]', () => { + const window = createProjectionWindow({ windowSize: 64 }); + const link = i => ({ a: `a${i}`, b: `b${i}`, risk: i / 10, level: 'clear', channels: { tree: i / 10, overlap: (10 - i) / 10, dependency: 0.3 } }); + const cold = projectPairs([link(1), link(2)], { window, minWindowForZscore: 8 }); + assert.strictEqual(cold.normalization, 'raw', 'two samples is below the warm-up size'); + assert.strictEqual(cold.window.samples, 2); + const warm = projectPairs(Array.from({ length: 10 }, (_, i) => link(i)), { window, minWindowForZscore: 8 }); + assert.strictEqual(warm.normalization, 'zscore-clipped'); + assert.strictEqual(warm.window.samples, 12); + assert.deepStrictEqual(warm.window.percentiles, [2.5, 97.5]); + assert.strictEqual(warm.window.channels[0].channel, 'x_tree'); + for (const pair of warm.pairs) { + for (const key of ['x_tree', 'x_overlap', 'x_dep']) { + assert.ok(pair.normalized[key] >= 0 && pair.normalized[key] <= 1, `${key} normalized within [0, 1]`); + } + } + const lowest = warm.pairs.find(p => p.a === 'a0'); + const highest = warm.pairs.find(p => p.a === 'a9'); + assert.ok(lowest.normalized.x_tree < highest.normalized.x_tree, 'ordering survives normalization'); + assert.strictEqual(warm.pairs[0].normalized.x_dep, 0.5, 'constant channel sits at 0.5'); +}); + +test('projectPairs: ignores malformed links and empty input', () => { + const out = projectPairs([null, { risk: 1 }, { a: 'x' }]); + assert.deepStrictEqual(out.pairs, []); + assert.deepStrictEqual(out.agents, []); + assert.deepStrictEqual(projectPairs(undefined).pairs, []); +}); + +test('scanAirspace links carry the per-channel values the projection needs', () => { + const agents = [ + { agentId: 'a', files: [{ path: 'src/api/users.js', lines: [[1, 50]] }] }, + { agentId: 'b', files: [{ path: 'src/api/users.js', lines: [[1, 50]] }] }, + { agentId: 'c', files: [{ path: 'docs/guide.md' }] } + ]; + const scan = scanAirspace(agents, {}); + assert.strictEqual(scan.links.length, 3); + for (const link of scan.links) { + assert.ok(link.channels, 'link has channels'); + for (const key of ['tree', 'overlap', 'dependency']) assert.ok(Number.isFinite(link.channels[key]), `${key} is numeric`); + } + const ab = scan.links.find(l => (l.a === 'a' && l.b === 'b') || (l.a === 'b' && l.b === 'a')); + assert.strictEqual(ab.channels.overlap, 1); + const out = projectPairs(scan.links); + assert.strictEqual(out.pairs.length, 3); + assert.strictEqual(out.agents.length, 3); +}); + +console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); +if (failed > 0) process.exit(1); diff --git a/tests/lib/control-plane-view-ui.test.js b/tests/lib/control-plane-view-ui.test.js new file mode 100644 index 000000000..2d0ab7880 --- /dev/null +++ b/tests/lib/control-plane-view-ui.test.js @@ -0,0 +1,51 @@ +'use strict'; + +const assert = require('assert'); +const vm = require('vm'); +const { renderControlPlaneViewHtml } = require('../../scripts/lib/control-pane/control-plane-view-ui'); + +async function renderResponse(ok, data) { + const elements = new Map(); + const context = new Proxy({}, { get: () => () => {} }); + function element() { + return { textContent: '', style: {}, appendChild() {}, getContext: () => context, + clientWidth: 640, clientHeight: 480, + parentElement: { getBoundingClientRect: () => ({ width: 640, height: 480 }) } }; + } + const document = { + getElementById(id) { if (!elements.has(id)) elements.set(id, element()); return elements.get(id); }, + createElement: element + }; + const html = renderControlPlaneViewHtml(); + const start = html.indexOf('', start); + assert.ok(start >= 0 && end > start, 'fixed renderer template must contain its inline script'); + const code = html.slice(start + '