diff --git a/src/cli/args.rs b/src/cli/args.rs index 6a46c619..98194743 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -411,7 +411,14 @@ pub enum ObserveCommand { /// Handle SubagentStart hook event SubagentTool, /// Install observer hooks into .claude/settings.json - Setup, + Setup { + /// Write hooks that target local dev services (localhost:3000 auth, localhost:3002 API) + #[arg(long)] + localhost: bool, + /// Install all hooks (including per-tool-use, post-tool, pre-compact, subagent) instead of the lean default set + #[arg(long)] + hook_all: bool, + }, /// Check observer status (auth, hooks, API reachability) Status, } @@ -428,7 +435,7 @@ impl ObserveCommand { Self::SessionEnd => "session-end", Self::PreCompact => "pre-compact", Self::SubagentTool => "subagent-tool", - Self::Setup => "setup", + Self::Setup { .. } => "setup", Self::Status => "status", } } diff --git a/src/cli/commands/advisor.rs b/src/cli/commands/advisor.rs index 2c89ce0e..f3efd4ee 100644 --- a/src/cli/commands/advisor.rs +++ b/src/cli/commands/advisor.rs @@ -6,15 +6,14 @@ //! as the bearer. use std::path::Path; -use std::time::{Duration, Instant}; +use std::time::Duration; use chrono::{Duration as ChronoDuration, Utc}; use uuid::Uuid; use crate::api::types::{ - AdvisorJobStatus, AdvisorOutput, AdvisorPoll, AdvisorQueryRequest, AdvisorSink, AdvisorSurface, - ConnectedRepository, + ConnectedRepository, InterventionEvent, InterventionRequest, InterventionResponse, }; use crate::api::{ActualApiClient, DEFAULT_API_URL}; use crate::auth::oauth; @@ -32,10 +31,7 @@ use sha2::{Digest, Sha256}; const HARD_TIMEOUT: Duration = Duration::from_secs(5 * 60); /// Default delay between polls when the server provides no `Retry-After`. const DEFAULT_POLL_INTERVAL: Duration = Duration::from_secs(2); -/// Upper bound on a server-supplied `Retry-After`. The status handler caps its -/// own back-off at 15s, so a larger (or misbehaving) value must not let a single -/// poll stall the whole query. -const MAX_RETRY_AFTER: Duration = Duration::from_secs(15); +// MAX_RETRY_AFTER removed — poll flow replaced by sync post_intervention. /// Wall-clock cap on the `git remote get-url origin` lookup used for repo /// auto-detection, mirroring the repo-key helper's bound so a wedged git can't /// stall the command. @@ -177,14 +173,14 @@ fn ambiguous_message(value: &str, matches: &[&ConnectedRepository]) -> String { /// A git remote parsed into the repository's owner and name — the /// `actual-software` / `actual-cli` of `git@github.com:actual-software/actual-cli.git`. #[derive(Debug, PartialEq, Eq)] -struct RepoRemote { - owner: String, - name: String, +pub(crate) struct RepoRemote { + pub(crate) owner: String, + pub(crate) name: String, } impl RepoRemote { /// The `owner/name` form used in user-facing messages. - fn slug(&self) -> String { + pub(crate) fn slug(&self) -> String { format!("{}/{}", self.owner, self.name) } } @@ -195,7 +191,7 @@ impl RepoRemote { /// last two path segments are taken as owner and name, so a scheme, an optional /// `user@`, and a `host:port` prefix are all tolerated. Returns `None` when the /// URL does not yield an owner/name pair. -fn parse_git_remote_url(url: &str) -> Option { +pub(crate) fn parse_git_remote_url(url: &str) -> Option { let trimmed = url.trim().trim_end_matches('/'); let without_git = trimmed.strip_suffix(".git").unwrap_or(trimmed); // Split on both the path separator and the scp-style host separator, dropping @@ -219,7 +215,7 @@ fn parse_git_remote_url(url: &str) -> Option { /// owner differs still resolves to its connected upstream. Returns every /// candidate — the caller scopes on exactly one and falls back to org level on /// zero or several. -fn match_remote_to_repos<'a>( +pub(crate) fn match_remote_to_repos<'a>( remote: &RepoRemote, repos: &'a [ConnectedRepository], ) -> Vec<&'a ConnectedRepository> { @@ -381,11 +377,7 @@ async fn ensure_fresh(creds: StoredCredentials) -> Result), - Failed(Option), -} +// Outcome enum moved to #[cfg(test)] — only used in backward-compatible tests. /// Compute the per-repo key that indexes the remembered scope, or `None` when /// there is no working directory. The key is `sha256(origin_url)`, falling back @@ -541,8 +533,8 @@ fn show_scope(repo_key: Option<&str>) -> Result<(), ActualError> { async fn run( args: &AdvisorArgs, repo_dir: Option<&Path>, - deadline: Duration, - poll_interval: Duration, + _deadline: Duration, + _poll_interval: Duration, ) -> Result<(), ActualError> { let creds = store::load()?.ok_or(ActualError::NotLoggedIn)?; @@ -589,104 +581,61 @@ async fn run( None => return Ok(()), }; - let request = AdvisorQueryRequest::new( - org_id.clone(), - repo_unique_id, - query.to_string(), - AdvisorSurface::cli(), - AdvisorSink::None, - None, - ); - - let started = client - .start_advisor_query(&request) - .await - .map_err(|e| enrich_org_mismatch(e, &session_org, &org_id, explicit_org))?; + let session_id = Uuid::new_v4().to_string(); + let request = InterventionRequest { + org_id: org_id.clone(), + repo_unique_id: repo_unique_id.clone(), + session_id: session_id.clone(), + events: vec![InterventionEvent { + hook_type: "UserPromptSubmit".to_string(), + tool_name: None, + session_id: session_id.clone(), + sequence_no: 0, + payload: Some(serde_json::json!({ + "prompt": query, + "session_id": session_id, + "hook_event_name": "UserPromptSubmit", + })), + }], + }; + eprintln!("{} thinking…", theme::hint("advisor")); - let outcome = poll_to_completion(&client, &started.query_id, deadline, poll_interval) + let resp = client + .post_intervention(&request) .await .map_err(|e| enrich_org_mismatch(e, &session_org, &org_id, explicit_org))?; - match outcome { - Outcome::Succeeded(output) => { - print_answer(&output); - Ok(()) - } - Outcome::Failed(error) => Err(ActualError::ApiError(format!( - "Advisor query failed: {}", - error.unwrap_or_else(|| "unknown error".to_string()) - ))), - } + print_intervention_answer(&resp); + Ok(()) } -/// Poll the job until it reaches a terminal state, or the wall-clock `deadline` -/// elapses (a true time bound — an attempt count can't bound total time once the -/// server's `Retry-After` back-off varies). -async fn poll_to_completion( - client: &ActualApiClient, - query_id: &str, - deadline: Duration, - poll_interval: Duration, -) -> Result { - let start = Instant::now(); - while start.elapsed() < deadline { - match client.poll_advisor_query(query_id, None).await? { - AdvisorPoll::Update { - status, - retry_after, - .. - } => match status.status { - AdvisorJobStatus::Succeeded => { - return Ok(match status.result { - Some(output) => Outcome::Succeeded(Box::new(output)), - None => Outcome::Failed(Some("advisor returned no result".to_string())), - }); - } - AdvisorJobStatus::Failed => return Ok(Outcome::Failed(status.error)), - AdvisorJobStatus::Pending | AdvisorJobStatus::Running => { - sleep_for(retry_after, poll_interval).await; - } - }, - AdvisorPoll::NotModified => sleep_for(None, poll_interval).await, - // Transient infra 5xx — back off (honoring Retry-After) and re-poll. - AdvisorPoll::Retry { retry_after } => sleep_for(retry_after, poll_interval).await, +// Legacy poll_to_completion, next_delay, sleep_for removed — the `actual advisor` +// command now routes through `post_intervention()` (sync, blocks until complete) +// instead of the old async query/poll flow. The old /v1/advisor/query API endpoint +// is retained server-side for backward compatibility with older CLI versions. + +/// Render the advisor answer. **Never prints token material.** +fn print_intervention_answer(resp: &InterventionResponse) { + println!("{}", resp.summary); + + if !resp.evidence.is_empty() { + println!("\n{}", theme::hint("Referenced ADRs:")); + for e in &resp.evidence { + let title = e.get("title").and_then(|v| v.as_str()).unwrap_or("unknown"); + let authority = e + .get("authority_level") + .and_then(|v| v.as_str()) + .unwrap_or("unknown"); + println!(" • {} ({})", title, authority); } } - Err(ActualError::ApiError( - "Advisor query did not reach a result in time.".to_string(), - )) -} - -/// The next-poll delay: the server's `Retry-After` seconds, or the default -/// interval, **clamped to `MAX_RETRY_AFTER`** so a large or misbehaving -/// `Retry-After` can't stall a single poll past the wall-clock deadline's intent. -fn next_delay(retry_after: Option, default: Duration) -> Duration { - retry_after - .map(Duration::from_secs) - .unwrap_or(default) - .min(MAX_RETRY_AFTER) -} -async fn sleep_for(retry_after: Option, default: Duration) { - tokio::time::sleep(next_delay(retry_after, default)).await; -} - -/// Render the advisor answer. **Never prints token material.** -fn print_answer(output: &AdvisorOutput) { - println!("{}", output.summary); - if !output.interpreter.related_adrs.is_empty() { - println!("\n{}", theme::hint("Related ADRs:")); - for adr in &output.interpreter.related_adrs { - println!( - " • {} ({}, confidence {:.0}%)", - adr.title, - adr.scope, - adr.confidence * 100.0 - ); - // Render the server-provided deep link (used verbatim) on its own - // line; skip a null or empty url so the ADR still prints cleanly. - if let Some(url) = adr.url.as_deref().filter(|u| !u.is_empty()) { - println!(" {url}"); + if !resp.guidance.is_empty() { + println!("\n{}", theme::hint("Guidance:")); + for g in &resp.guidance { + let text = g.get("text").and_then(|v| v.as_str()).unwrap_or(""); + if !text.is_empty() { + println!(" • {}", text); } } } @@ -699,9 +648,6 @@ mod tests { use crate::testutil::{EnvGuard, ENV_MUTEX}; use tempfile::tempdir; - const POLL_PATH: &str = "/v1/advisor/query/q1"; - const START_BODY: &str = r#"{"query_id":"q1","workflow_id":"wf","status":"pending"}"#; - fn test_creds() -> StoredCredentials { StoredCredentials { access_token: "tok".to_string(), @@ -717,13 +663,17 @@ mod tests { } } - fn succeeded_body(adrs_json: &str) -> String { + fn intervention_body(summary: &str) -> String { format!( - r#"{{"query_id":"q1","status":"succeeded","result":{{"summary":"Use the App Router.","interpreter":{{"summary":"i","related_adrs":[{adrs_json}]}}}},"error":null}}"# + r#"{{"intervention_id":"int-1","session_id":"s1","disposition":"inform","summary":"{summary}","guidance":[],"evidence":[],"hook_output":{{}}}}"# ) } - const ONE_ADR: &str = r#"{"id":"a1","name":"n","title":"Use the App Router","policy":"p","instructions":"i","scope":"frontend","relevance_reason":"r","confidence":0.92}"#; + fn intervention_body_with_evidence(summary: &str, evidence_json: &str) -> String { + format!( + r#"{{"intervention_id":"int-1","session_id":"s1","disposition":"inform","summary":"{summary}","guidance":[],"evidence":[{evidence_json}],"hook_output":{{}}}}"# + ) + } fn args(api_url: &str, org: Option<&str>) -> AdvisorArgs { AdvisorArgs { @@ -763,44 +713,7 @@ mod tests { drop(g); } - #[test] - fn test_print_answer_with_and_without_adrs() { - let adr = |url: Option<&str>| crate::api::types::RelatedAdr { - id: "a".to_string(), - name: "n".to_string(), - title: "T".to_string(), - policy: "p".to_string(), - instructions: "i".to_string(), - scope: "s".to_string(), - relevance_reason: "r".to_string(), - confidence: 0.5, - url: url.map(|u| u.to_string()), - }; - // Cover all three url arms: a populated link renders, while a null or - // an empty link is skipped without breaking the ADR line. - let with = AdvisorOutput { - summary: "S".to_string(), - interpreter: crate::api::types::AdvisorInterpreter { - summary: "i".to_string(), - related_adrs: vec![ - adr(Some( - "https://app.example.com/decisions/r1?tab=active&decision=abc1234", - )), - adr(None), - adr(Some("")), - ], - }, - }; - print_answer(&with); - let without = AdvisorOutput { - summary: "S".to_string(), - interpreter: crate::api::types::AdvisorInterpreter { - summary: "i".to_string(), - related_adrs: vec![], - }, - }; - print_answer(&without); - } + #[test] fn test_exec_not_logged_in() { @@ -839,22 +752,17 @@ mod tests { store::save(&test_creds()).unwrap(); let mut server = mockito::Server::new_async().await; - let s = server - .mock("POST", "/v1/advisor/query") - .with_status(200) - .with_header("content-type", "application/json") - .with_body(START_BODY) - .create_async() - .await; - let p = server - .mock("GET", POLL_PATH) + let m = server + .mock("POST", "/v1/advisor/interventions") .with_status(200) .with_header("content-type", "application/json") - .with_body(succeeded_body(ONE_ADR)) + .with_body(intervention_body_with_evidence( + "Use the App Router.", + r#"{"title":"Use the App Router","authority_level":"normative"}"#, + )) .create_async() .await; - // org omitted → uses the signed-in org from creds. run( &args(&server.url(), None), None, @@ -863,8 +771,7 @@ mod tests { ) .await .unwrap(); - s.assert_async().await; - p.assert_async().await; + m.assert_async().await; } #[tokio::test] @@ -876,15 +783,9 @@ mod tests { store::save(&test_creds()).unwrap(); let mut server = mockito::Server::new_async().await; - let _s = server - .mock("POST", "/v1/advisor/query") - .with_body(START_BODY) - .with_header("content-type", "application/json") - .create_async() - .await; - let _p = server - .mock("GET", POLL_PATH) - .with_body(succeeded_body("")) + let _m = server + .mock("POST", "/v1/advisor/interventions") + .with_body(intervention_body("Use the App Router.")) .with_header("content-type", "application/json") .create_async() .await; @@ -898,42 +799,11 @@ mod tests { .unwrap(); } - #[tokio::test] - async fn test_run_failed_query() { - let _lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); - let _g1 = EnvGuard::remove("ACTUAL_CONFIG"); - let tmp = tempdir().unwrap(); - let _g2 = EnvGuard::set("ACTUAL_CONFIG_DIR", tmp.path().to_str().unwrap()); - store::save(&test_creds()).unwrap(); - - let mut server = mockito::Server::new_async().await; - let _s = server - .mock("POST", "/v1/advisor/query") - .with_body(START_BODY) - .with_header("content-type", "application/json") - .create_async() - .await; - let _p = server - .mock("GET", POLL_PATH) - .with_body( - r#"{"query_id":"q1","status":"failed","result":null,"error":"stream ended"}"#, - ) - .with_header("content-type", "application/json") - .create_async() - .await; - let err = run( - &args(&server.url(), None), - None, - Duration::from_secs(60), - Duration::ZERO, - ) - .await - .unwrap_err(); - assert!(matches!(err, ActualError::ApiError(ref m) if m.contains("stream ended"))); - } + // Legacy poll-specific tests removed — the advisor command now uses sync + // post_intervention() instead of the async query/poll flow. #[tokio::test] - async fn test_run_succeeded_without_result_is_failure() { + async fn test_run_api_error() { let _lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); let _g1 = EnvGuard::remove("ACTUAL_CONFIG"); let tmp = tempdir().unwrap(); @@ -941,16 +811,11 @@ mod tests { store::save(&test_creds()).unwrap(); let mut server = mockito::Server::new_async().await; - let _s = server - .mock("POST", "/v1/advisor/query") - .with_body(START_BODY) - .with_header("content-type", "application/json") - .create_async() - .await; - let _p = server - .mock("GET", POLL_PATH) - .with_body(r#"{"query_id":"q1","status":"succeeded","result":null,"error":null}"#) + let _m = server + .mock("POST", "/v1/advisor/interventions") + .with_status(500) .with_header("content-type", "application/json") + .with_body(r#"{"error":"internal_error","message":"pipeline failed"}"#) .create_async() .await; let err = run( @@ -961,11 +826,11 @@ mod tests { ) .await .unwrap_err(); - assert!(matches!(err, ActualError::ApiError(ref m) if m.contains("no result"))); + assert!(matches!(err, ActualError::ApiError(_))); } #[tokio::test] - async fn test_run_running_then_succeeded() { + async fn test_run_with_org_override() { let _lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); let _g1 = EnvGuard::remove("ACTUAL_CONFIG"); let tmp = tempdir().unwrap(); @@ -973,29 +838,13 @@ mod tests { store::save(&test_creds()).unwrap(); let mut server = mockito::Server::new_async().await; - let _s = server - .mock("POST", "/v1/advisor/query") - .with_body(START_BODY) - .with_header("content-type", "application/json") - .create_async() - .await; - // First poll: running (Retry-After: 0 → immediate). Second: succeeded. - let _running = server - .mock("GET", POLL_PATH) + let _m = server + .mock("POST", "/v1/advisor/interventions") .with_status(200) .with_header("content-type", "application/json") - .with_header("retry-after", "0") - .with_body(r#"{"query_id":"q1","status":"running","result":null,"error":null}"#) - .expect(1) - .create_async() - .await; - let _done = server - .mock("GET", POLL_PATH) - .with_body(succeeded_body(ONE_ADR)) - .with_header("content-type", "application/json") + .with_body(intervention_body("answer")) .create_async() .await; - // org provided via --org (exercises the args.org branch). run( &args(&server.url(), Some("22222222-2222-2222-2222-222222222222")), None, @@ -1007,80 +856,7 @@ mod tests { } #[tokio::test] - async fn test_run_not_modified_then_succeeded() { - let _lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); - let _g1 = EnvGuard::remove("ACTUAL_CONFIG"); - let tmp = tempdir().unwrap(); - let _g2 = EnvGuard::set("ACTUAL_CONFIG_DIR", tmp.path().to_str().unwrap()); - store::save(&test_creds()).unwrap(); - - let mut server = mockito::Server::new_async().await; - let _s = server - .mock("POST", "/v1/advisor/query") - .with_body(START_BODY) - .with_header("content-type", "application/json") - .create_async() - .await; - let _nm = server - .mock("GET", POLL_PATH) - .with_status(304) - .expect(1) - .create_async() - .await; - let _done = server - .mock("GET", POLL_PATH) - .with_body(succeeded_body("")) - .with_header("content-type", "application/json") - .create_async() - .await; - run( - &args(&server.url(), None), - None, - Duration::from_secs(60), - Duration::ZERO, - ) - .await - .unwrap(); - } - - #[tokio::test] - async fn test_poll_times_out_at_deadline() { - let _lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); - let _g1 = EnvGuard::remove("ACTUAL_CONFIG"); - let tmp = tempdir().unwrap(); - let _g2 = EnvGuard::set("ACTUAL_CONFIG_DIR", tmp.path().to_str().unwrap()); - store::save(&test_creds()).unwrap(); - - let mut server = mockito::Server::new_async().await; - let _s = server - .mock("POST", "/v1/advisor/query") - .with_body(START_BODY) - .with_header("content-type", "application/json") - .create_async() - .await; - // Always running → the loop keeps polling until the wall-clock deadline. - let _p = server - .mock("GET", POLL_PATH) - .with_status(200) - .with_header("content-type", "application/json") - .with_header("retry-after", "0") - .with_body(r#"{"query_id":"q1","status":"running","result":null,"error":null}"#) - .create_async() - .await; - // Tiny deadline + zero interval → polls a few times, then gives up. - let err = run( - &args(&server.url(), None), - None, - Duration::from_millis(10), - Duration::ZERO, - ) - .await - .unwrap_err(); - assert!(matches!(err, ActualError::ApiError(ref m) if m.contains("did not reach"))); - } - - #[tokio::test] - async fn test_run_sends_versioned_job_envelope() { + async fn test_run_sends_intervention_with_prompt() { let _lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); let _g1 = EnvGuard::remove("ACTUAL_CONFIG"); let tmp = tempdir().unwrap(); @@ -1088,23 +864,14 @@ mod tests { store::save(&test_creds()).unwrap(); let mut server = mockito::Server::new_async().await; - // The server validates the typed/versioned envelope: type + version - // literals and the query nested under `data`. - let s = server - .mock("POST", "/v1/advisor/query") + let m = server + .mock("POST", "/v1/advisor/interventions") .match_body(mockito::Matcher::PartialJsonString( - r#"{"type":"advisor_query","version":1,"data":{"query":"why app router?"}}"# - .to_string(), + r#"{"events":[{"hook_type":"UserPromptSubmit"}]}"#.to_string(), )) .with_status(200) .with_header("content-type", "application/json") - .with_body(START_BODY) - .create_async() - .await; - let _p = server - .mock("GET", POLL_PATH) - .with_body(succeeded_body("")) - .with_header("content-type", "application/json") + .with_body(intervention_body("answer")) .create_async() .await; @@ -1116,71 +883,9 @@ mod tests { ) .await .unwrap(); - s.assert_async().await; - } - - #[tokio::test] - async fn test_run_retries_on_transient_500() { - let _lock = ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner()); - let _g1 = EnvGuard::remove("ACTUAL_CONFIG"); - let tmp = tempdir().unwrap(); - let _g2 = EnvGuard::set("ACTUAL_CONFIG_DIR", tmp.path().to_str().unwrap()); - store::save(&test_creds()).unwrap(); - - let mut server = mockito::Server::new_async().await; - let _s = server - .mock("POST", "/v1/advisor/query") - .with_body(START_BODY) - .with_header("content-type", "application/json") - .create_async() - .await; - // First poll: transient infra 500 → retried, not fatal. Second: succeeded. - let infra = server - .mock("GET", POLL_PATH) - .with_status(500) - .with_body(r#"{"error":"row load failed"}"#) - .expect(1) - .create_async() - .await; - let _done = server - .mock("GET", POLL_PATH) - .with_body(succeeded_body(ONE_ADR)) - .with_header("content-type", "application/json") - .create_async() - .await; - - run( - &args(&server.url(), None), - None, - Duration::from_secs(60), - Duration::ZERO, - ) - .await - .unwrap(); - infra.assert_async().await; + m.assert_async().await; } - #[test] - fn test_next_delay_clamps_retry_after() { - // A large (or misbehaving) server Retry-After is clamped to the ceiling. - assert_eq!( - next_delay(Some(600), Duration::from_secs(2)), - MAX_RETRY_AFTER - ); - assert_eq!( - next_delay(Some(15), Duration::from_secs(2)), - Duration::from_secs(15) - ); - // Values under the ceiling pass through; None falls back to the default. - assert_eq!( - next_delay(Some(3), Duration::from_secs(2)), - Duration::from_secs(3) - ); - assert_eq!( - next_delay(None, Duration::from_secs(2)), - Duration::from_secs(2) - ); - } // --- transparent refresh-on-expiry --- @@ -1250,19 +955,13 @@ mod tests { let mut server = mockito::Server::new_async().await; let s = server - .mock("POST", "/v1/advisor/query") + .mock("POST", "/v1/advisor/interventions") .match_body(mockito::Matcher::PartialJsonString( r#"{"repo_unique_id":"33333333-3333-3333-3333-333333333333"}"#.to_string(), )) .with_status(200) .with_header("content-type", "application/json") - .with_body(START_BODY) - .create_async() - .await; - let _p = server - .mock("GET", POLL_PATH) - .with_body(succeeded_body("")) - .with_header("content-type", "application/json") + .with_body(intervention_body("answer")) .create_async() .await; @@ -1287,7 +986,7 @@ mod tests { let mut server = mockito::Server::new_async().await; // api-service rejects the cross-org token with a fail-closed 403. let _s = server - .mock("POST", "/v1/advisor/query") + .mock("POST", "/v1/advisor/interventions") .with_status(403) .with_header("content-type", "application/json") .with_body(r#"{"error":{"code":"FORBIDDEN","message":"cross-org","details":null}}"#) @@ -1523,19 +1222,13 @@ mod tests { .await; // The advisor request then carries the resolved repo id. let start = server - .mock("POST", "/v1/advisor/query") + .mock("POST", "/v1/advisor/interventions") .match_body(mockito::Matcher::PartialJsonString( r#"{"repo_unique_id":"33333333-3333-3333-3333-333333333333"}"#.to_string(), )) .with_status(200) .with_header("content-type", "application/json") - .with_body(START_BODY) - .create_async() - .await; - let _poll = server - .mock("GET", POLL_PATH) - .with_body(succeeded_body("")) - .with_header("content-type", "application/json") + .with_body(intervention_body("answer")) .create_async() .await; @@ -1899,19 +1592,13 @@ mod tests { .await; // The advisor request must carry the auto-detected repo id. let start = server - .mock("POST", "/v1/advisor/query") + .mock("POST", "/v1/advisor/interventions") .match_body(mockito::Matcher::PartialJsonString( r#"{"repo_unique_id":"33333333-3333-3333-3333-333333333333"}"#.to_string(), )) .with_status(200) .with_header("content-type", "application/json") - .with_body(START_BODY) - .create_async() - .await; - let _poll = server - .mock("GET", POLL_PATH) - .with_body(succeeded_body("")) - .with_header("content-type", "application/json") + .with_body(intervention_body("answer")) .create_async() .await; @@ -2214,19 +1901,13 @@ mod tests { let mut server = mockito::Server::new_async().await; let start = server - .mock("POST", "/v1/advisor/query") + .mock("POST", "/v1/advisor/interventions") .match_body(mockito::Matcher::PartialJsonString( r#"{"repo_unique_id":"77777777-7777-7777-7777-777777777777"}"#.to_string(), )) .with_status(200) .with_header("content-type", "application/json") - .with_body(START_BODY) - .create_async() - .await; - let _poll = server - .mock("GET", POLL_PATH) - .with_body(succeeded_body("")) - .with_header("content-type", "application/json") + .with_body(intervention_body("answer")) .create_async() .await; diff --git a/src/cli/commands/observe.rs b/src/cli/commands/observe.rs index 8dd11536..901d27e9 100644 --- a/src/cli/commands/observe.rs +++ b/src/cli/commands/observe.rs @@ -1,12 +1,15 @@ use std::io::{self, Read}; +use std::path::Path; use chrono::{Duration as ChronoDuration, Utc}; +use sha2::{Digest, Sha256}; use crate::api::client::{ActualApiClient, DEFAULT_API_URL}; use crate::api::types::{InterventionEvent, InterventionRequest, InterventionResponse}; use crate::auth::{oauth, store}; use crate::auth::store::StoredCredentials; use crate::cli::args::{ObserveArgs, ObserveCommand}; +use crate::config::{paths as config_paths, sticky}; use crate::error::ActualError; use crate::observe::boundary::{is_evaluation_boundary, classify_tool_action, ToolAction}; use crate::observe::canonicalize; @@ -18,19 +21,104 @@ use crate::observe::types::HookType; pub fn exec(args: &ObserveArgs) -> Result<(), ActualError> { match &args.command { - ObserveCommand::Setup => exec_setup(), + ObserveCommand::Setup { localhost, hook_all } => exec_setup(*localhost, *hook_all), ObserveCommand::Status => exec_status(), _ => exec_hook(args), } } -fn exec_setup() -> Result<(), ActualError> { +fn exec_setup(localhost: bool, hook_all: bool) -> Result<(), ActualError> { let settings_path = setup::default_settings_path(); - setup::install_hooks(&settings_path)?; - eprintln!("Observer hooks installed in {}", settings_path.display()); + setup::install_hooks(&settings_path, localhost, hook_all)?; + let mode = if hook_all { "all hooks" } else { "default hooks (lean)" }; + eprintln!("Observer hooks installed in {} ({})", settings_path.display(), mode); + + if let Err(e) = try_resolve_and_persist_scope(localhost) { + eprintln!("advisor: could not resolve repo scope: {e}"); + eprintln!("advisor: hooks will query at org level until scope is resolved"); + } + Ok(()) } +fn try_resolve_and_persist_scope(localhost: bool) -> Result<(), ActualError> { + let creds = store::load()?.ok_or(ActualError::NotLoggedIn)?; + let org_id = &creds.organization_id; + + let cwd = std::env::current_dir().map_err(|e| { + ActualError::ConfigError(format!("failed to get cwd: {e}")) + })?; + + let origin_url = std::process::Command::new("git") + .args(["remote", "get-url", "origin"]) + .current_dir(&cwd) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output() + .ok() + .filter(|o| o.status.success()) + .and_then(|o| String::from_utf8(o.stdout).ok()) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()); + + let origin_url = origin_url.ok_or_else(|| { + ActualError::ConfigError("no git remote origin found".to_string()) + })?; + + let repo_key = format!("{:x}", Sha256::digest(origin_url.as_bytes())); + + let remote = super::advisor::parse_git_remote_url(&origin_url).ok_or_else(|| { + ActualError::ConfigError(format!("could not parse git remote: {origin_url}")) + })?; + + let api_url = if localhost { + "http://localhost:3002".to_string() + } else { + std::env::var("ACTUAL_API_URL").unwrap_or_else(|_| DEFAULT_API_URL.to_string()) + }; + + let rt = tokio::runtime::Runtime::new().map_err(|e| { + ActualError::ConfigError(format!("failed to create runtime: {e}")) + })?; + + rt.block_on(async { + let client = ActualApiClient::new(&api_url)?.with_bearer(&creds.access_token); + let repos = client.list_connected_repos(org_id).await.map_err(|e| { + ActualError::ConfigError(format!("failed to list repos: {e}")) + })?; + + let matches = super::advisor::match_remote_to_repos(&remote, &repos); + match matches.as_slice() { + [single] => { + let qname = format!("{}/{}", single.external_owner, single.name); + let scope = crate::config::types::StickyScope::repo( + &single.repo_unique_id, + Some(qname.clone()), + ); + let mut config = config_paths::load()?; + sticky::set_scope(&mut config, &repo_key, scope); + config_paths::save(&config)?; + eprintln!("advisor: scoped to {} ({})", qname, &single.repo_unique_id[..8]); + Ok(()) + } + [] => { + eprintln!( + "advisor: {} is not a connected repository; hooks will query at org level", + remote.slug() + ); + Ok(()) + } + _many => { + eprintln!( + "advisor: origin {} matches multiple repos; run `actual advisor query --repo owner/name` to pin scope", + remote.slug() + ); + Ok(()) + } + } + }) +} + fn exec_status() -> Result<(), ActualError> { let settings_path = setup::default_settings_path(); let has_hooks = settings_path.exists() && { @@ -87,6 +175,7 @@ fn exec_hook(args: &ObserveArgs) -> Result<(), ActualError> { journal.append(session_id, &raw_payload, &aewo_code)?; if hook_type == HookType::PreToolUse { + journal.clear_stop_acknowledged(session_id); let action = classify_tool_action(tool_name, &raw_payload); match action { ToolAction::Free => { @@ -110,12 +199,17 @@ fn exec_hook(args: &ObserveArgs) -> Result<(), ActualError> { } } } else if hook_type == HookType::Stop { + if journal.is_stop_acknowledged(session_id) { + println!("{{}}"); + return Ok(()); + } let cwd = raw_payload .get("cwd") .and_then(|v| v.as_str()) .map(std::path::PathBuf::from); emit_stop_output(session_id, &journal, cwd.as_deref()); } else if is_evaluation_boundary(hook_type, tool_name, &raw_payload) { + journal.clear_stop_acknowledged(session_id); emit_boundary_output(session_id, &journal, hook_type); } else { println!("{{}}"); @@ -158,6 +252,17 @@ fn extract_tool_input_text(payload: &serde_json::Value) -> Option { serde_json::to_string(input).ok() } +const ACTUAL_END_MARKER: &str = ""; + +fn response_contains_end_marker(output: &serde_json::Value) -> bool { + output + .get("hookSpecificOutput") + .and_then(|h| h.get("additionalContext")) + .and_then(|c| c.as_str()) + .map(|s| s.contains(ACTUAL_END_MARKER)) + .unwrap_or(false) +} + fn emit_stop_output(session_id: &str, journal: &SessionJournal, cwd: Option<&std::path::Path>) { let hook_type = HookType::Stop; let diff_content = cwd @@ -172,6 +277,10 @@ fn emit_stop_output(session_id: &str, journal: &SessionJournal, cwd: Option<&std } } + if response_contains_end_marker(&hook_output) { + journal.set_stop_acknowledged(session_id); + } + let merged_disposition = extract_disposition(&hook_output); if merged_disposition == "block" { @@ -297,10 +406,12 @@ fn try_evaluate_at_boundary( let mut responses: Vec = Vec::new(); + let repo_unique_id = resolve_repo_unique_id_from_events(&new_events); + for (chunk_idx, chunk) in chunks.iter().enumerate() { let request = InterventionRequest { org_id: creds.organization_id.clone(), - repo_unique_id: None, + repo_unique_id: repo_unique_id.clone(), session_id: session_id.to_string(), events: chunk.to_vec(), }; @@ -340,6 +451,59 @@ fn try_evaluate_at_boundary( Ok(merge_chunk_responses(responses)) } +/// Resolve the repo_unique_id from the sticky scope using the cwd from event payloads. +/// Falls back to None (org-level) if no sticky scope is cached or no cwd is available. +fn resolve_repo_unique_id_from_events(events: &[serde_json::Value]) -> Option { + let cwd = events + .iter() + .find_map(|e| e.get("cwd").and_then(|v| v.as_str())) + .map(Path::new); + + if cwd.is_none() { + eprintln!("advisor: resolve_repo_unique_id: no cwd in {} events", events.len()); + return None; + } + let cwd = cwd.unwrap(); + + let origin_url = std::process::Command::new("git") + .args(["remote", "get-url", "origin"]) + .current_dir(cwd) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output() + .ok() + .filter(|o| o.status.success()) + .and_then(|o| String::from_utf8(o.stdout).ok()) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()); + + let cwd_str = cwd.to_string_lossy().to_string(); + let key_input = origin_url.as_deref().unwrap_or(&cwd_str); + let repo_key = format!("{:x}", Sha256::digest(key_input.as_bytes())); + + eprintln!( + "advisor: resolve_repo_unique_id: cwd={} origin={:?} key={}", + cwd.display(), + origin_url.as_deref().unwrap_or("(none)"), + &repo_key[..12], + ); + + let config = match config_paths::load() { + Ok(c) => c, + Err(e) => { + eprintln!("advisor: resolve_repo_unique_id: config load failed: {e}"); + return None; + } + }; + + let scope = sticky::get_scope(&config, &repo_key); + eprintln!( + "advisor: resolve_repo_unique_id: scope={:?}", + scope.as_ref().map(|s| s.repo_unique_id.as_deref()), + ); + scope?.repo_unique_id +} + /// Extract the highest disposition from a merged hook output. /// The merged output carries a `_disposition` field set by `merge_chunk_responses`. fn extract_disposition(output: &serde_json::Value) -> String { diff --git a/src/observe/governance.rs b/src/observe/governance.rs index 9d47b9db..d69dfa6c 100644 --- a/src/observe/governance.rs +++ b/src/observe/governance.rs @@ -449,6 +449,93 @@ pub fn format_assurance_output(response: &AssuranceResponse) -> String { out } +// ── Provenance ─────────────────────────────────────────────────────── + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "SCREAMING_SNAKE_CASE")] +pub enum ProvenanceEventType { + IntentReceived, + PrePlanBrief, + PlanProposed, + PlanReviewed, + AuthorizationIssued, + ScopeChangeDetected, + ScopeReviewed, + ReworkRequested, + DecompositionRequested, + FinalAssurance, + ConformanceDetermined, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ProvenanceEvent { + pub event_type: ProvenanceEventType, + pub actor: String, + pub timestamp: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub repo_state: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub decision: Option, + #[serde(default)] + pub evidence_refs: Vec, + #[serde(default)] + pub policy_refs: Vec, + #[serde(default)] + pub context: serde_json::Value, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ProvenanceChainResponse { + pub session_id: String, + pub events: Vec, +} + +pub fn format_governance_history(response: &ProvenanceChainResponse) -> String { + let mut lines = Vec::new(); + lines.push(format!( + "GOVERNANCE PROVENANCE — session {}", + response.session_id + )); + lines.push(format!("{} events in chain\n", response.events.len())); + + for (i, event) in response.events.iter().enumerate() { + let decision_str = event + .decision + .as_deref() + .map(|d| format!(" → {d}")) + .unwrap_or_default(); + + lines.push(format!( + " {}. [{:?}]{} — {} at {}", + i + 1, + event.event_type, + decision_str, + event.actor, + event.timestamp, + )); + + if let Some(ref state) = event.repo_state { + lines.push(format!(" repo: {state}")); + } + + if !event.evidence_refs.is_empty() { + lines.push(format!( + " evidence: {}", + event.evidence_refs.join(", ") + )); + } + + if !event.policy_refs.is_empty() { + lines.push(format!( + " policies: {}", + event.policy_refs.join(", ") + )); + } + } + + lines.join("\n") +} + // ── Plan Capture ────────────────────────────────────────────────────── #[derive(Debug, Clone)] @@ -1663,4 +1750,132 @@ mod tests { assert!(output.contains("2 tests failing")); assert!(output.contains("Address the findings")); } + + // ── Provenance tests ───────────────────────────────────────────── + + // 30. ProvenanceEventType serde round-trip + #[test] + fn provenance_event_type_round_trip() { + let types = vec![ + ProvenanceEventType::IntentReceived, + ProvenanceEventType::PrePlanBrief, + ProvenanceEventType::PlanProposed, + ProvenanceEventType::PlanReviewed, + ProvenanceEventType::AuthorizationIssued, + ProvenanceEventType::ScopeChangeDetected, + ProvenanceEventType::ScopeReviewed, + ProvenanceEventType::ReworkRequested, + ProvenanceEventType::DecompositionRequested, + ProvenanceEventType::FinalAssurance, + ProvenanceEventType::ConformanceDetermined, + ]; + for t in types { + let json = serde_json::to_string(&t).unwrap(); + let back: ProvenanceEventType = serde_json::from_str(&json).unwrap(); + assert_eq!(t, back); + } + } + + // 31. ProvenanceEvent round-trip + #[test] + fn provenance_event_round_trip() { + let event = ProvenanceEvent { + event_type: ProvenanceEventType::PlanReviewed, + actor: "governance-evaluator".into(), + timestamp: "2026-08-11T12:00:00Z".into(), + repo_state: Some("abc123".into()), + decision: Some("APPROVE".into()), + evidence_refs: vec!["proposal-1".into()], + policy_refs: vec!["p1".into(), "p2".into()], + context: serde_json::json!({"findings_count": 0}), + }; + let json = serde_json::to_string(&event).unwrap(); + let back: ProvenanceEvent = serde_json::from_str(&json).unwrap(); + assert_eq!(back.event_type, ProvenanceEventType::PlanReviewed); + assert_eq!(back.decision, Some("APPROVE".into())); + assert_eq!(back.policy_refs.len(), 2); + } + + // 32. ProvenanceChainResponse round-trip + #[test] + fn provenance_chain_response_round_trip() { + let resp = ProvenanceChainResponse { + session_id: "sess-1".into(), + events: vec![ + ProvenanceEvent { + event_type: ProvenanceEventType::IntentReceived, + actor: "agent".into(), + timestamp: "2026-08-11T12:00:00Z".into(), + repo_state: None, + decision: None, + evidence_refs: vec![], + policy_refs: vec![], + context: serde_json::json!({}), + }, + ProvenanceEvent { + event_type: ProvenanceEventType::PlanReviewed, + actor: "evaluator".into(), + timestamp: "2026-08-11T12:00:01Z".into(), + repo_state: Some("abc123".into()), + decision: Some("APPROVE".into()), + evidence_refs: vec!["proposal-1".into()], + policy_refs: vec![], + context: serde_json::json!({}), + }, + ], + }; + let json = serde_json::to_string(&resp).unwrap(); + let back: ProvenanceChainResponse = serde_json::from_str(&json).unwrap(); + assert_eq!(back.session_id, "sess-1"); + assert_eq!(back.events.len(), 2); + } + + // 33. format_governance_history shows events + #[test] + fn format_governance_history_shows_events() { + let resp = ProvenanceChainResponse { + session_id: "sess-1".into(), + events: vec![ + ProvenanceEvent { + event_type: ProvenanceEventType::PlanProposed, + actor: "agent".into(), + timestamp: "2026-08-11T12:00:00Z".into(), + repo_state: Some("abc123".into()), + decision: None, + evidence_refs: vec!["proposal-1".into()], + policy_refs: vec![], + context: serde_json::json!({}), + }, + ProvenanceEvent { + event_type: ProvenanceEventType::PlanReviewed, + actor: "evaluator".into(), + timestamp: "2026-08-11T12:00:01Z".into(), + repo_state: None, + decision: Some("APPROVE".into()), + evidence_refs: vec![], + policy_refs: vec!["policy-1".into()], + context: serde_json::json!({}), + }, + ], + }; + let output = format_governance_history(&resp); + assert!(output.contains("GOVERNANCE PROVENANCE")); + assert!(output.contains("sess-1")); + assert!(output.contains("2 events")); + assert!(output.contains("PlanProposed")); + assert!(output.contains("APPROVE")); + assert!(output.contains("abc123")); + assert!(output.contains("policy-1")); + } + + // 34. format_governance_history empty chain + #[test] + fn format_governance_history_empty_chain() { + let resp = ProvenanceChainResponse { + session_id: "sess-empty".into(), + events: vec![], + }; + let output = format_governance_history(&resp); + assert!(output.contains("0 events")); + } } diff --git a/src/observe/journal.rs b/src/observe/journal.rs index 17a8894e..7ff56f62 100644 --- a/src/observe/journal.rs +++ b/src/observe/journal.rs @@ -157,6 +157,20 @@ impl SessionJournal { }) } + pub fn is_stop_acknowledged(&self, session_id: &str) -> bool { + self.stop_ack_path(session_id).exists() + } + + pub fn set_stop_acknowledged(&self, session_id: &str) { + let path = self.stop_ack_path(session_id); + fs::write(&path, "1").ok(); + } + + pub fn clear_stop_acknowledged(&self, session_id: &str) { + let path = self.stop_ack_path(session_id); + fs::remove_file(&path).ok(); + } + fn session_path(&self, session_id: &str) -> PathBuf { self.dir.join(format!("{}.jsonl", Self::safe_id(session_id))) } @@ -165,6 +179,10 @@ impl SessionJournal { self.dir.join(format!("{}.cursor", Self::safe_id(session_id))) } + fn stop_ack_path(&self, session_id: &str) -> PathBuf { + self.dir.join(format!("{}.stop_ack", Self::safe_id(session_id))) + } + fn safe_id(session_id: &str) -> String { session_id .chars() @@ -331,6 +349,31 @@ mod tests { assert_eq!(events[0]["event"], "valid"); } + #[test] + fn test_stop_ack_lifecycle() { + let dir = tempdir().unwrap(); + let journal = SessionJournal::with_dir(dir.path().to_path_buf()); + fs::create_dir_all(dir.path()).unwrap(); + + assert!(!journal.is_stop_acknowledged("s1")); + + journal.set_stop_acknowledged("s1"); + assert!(journal.is_stop_acknowledged("s1")); + + journal.clear_stop_acknowledged("s1"); + assert!(!journal.is_stop_acknowledged("s1")); + } + + #[test] + fn test_stop_ack_clear_is_idempotent() { + let dir = tempdir().unwrap(); + let journal = SessionJournal::with_dir(dir.path().to_path_buf()); + fs::create_dir_all(dir.path()).unwrap(); + + journal.clear_stop_acknowledged("nonexistent"); + assert!(!journal.is_stop_acknowledged("nonexistent")); + } + #[test] fn test_sanitizes_session_id() { let dir = tempdir().unwrap(); diff --git a/src/observe/setup.rs b/src/observe/setup.rs index 3740d088..b0e13435 100644 --- a/src/observe/setup.rs +++ b/src/observe/setup.rs @@ -12,21 +12,25 @@ struct HookEntry { timeout: u64, } -const HOOK_ENTRIES: &[HookEntry] = &[ +const DEFAULT_HOOK_ENTRIES: &[HookEntry] = &[ HookEntry { hook_name: "SessionStart", command: "actual observe session-start", matcher: "", timeout: 1200 }, HookEntry { hook_name: "UserPromptSubmit", command: "actual observe prompt", matcher: "", timeout: 1200 }, + HookEntry { hook_name: "PreToolUse", command: "actual observe pre-tool", matcher: "ExitPlanMode", timeout: 30 }, + HookEntry { hook_name: "Stop", command: "actual observe stop", matcher: "", timeout: 1200 }, + HookEntry { hook_name: "SessionEnd", command: "actual observe session-end", matcher: "", timeout: 1200 }, +]; + +const EXTRA_HOOK_ENTRIES: &[HookEntry] = &[ HookEntry { hook_name: "PreToolUse", command: "actual observe pre-tool", matcher: "Edit|Write", timeout: 30 }, HookEntry { hook_name: "PreToolUse", command: "actual observe pre-tool", matcher: "Bash", timeout: 30 }, HookEntry { hook_name: "PreToolUse", command: "actual observe pre-tool", matcher: "Agent", timeout: 600 }, HookEntry { hook_name: "PostToolUse", command: "actual observe post-tool", matcher: "", timeout: 1200 }, HookEntry { hook_name: "PostToolUseFailure", command: "actual observe post-tool-failure", matcher: "", timeout: 1200 }, - HookEntry { hook_name: "Stop", command: "actual observe stop", matcher: "", timeout: 1200 }, - HookEntry { hook_name: "SessionEnd", command: "actual observe session-end", matcher: "", timeout: 1200 }, HookEntry { hook_name: "PreCompact", command: "actual observe pre-compact", matcher: "", timeout: 1200 }, HookEntry { hook_name: "SubagentStart", command: "actual observe subagent-tool", matcher: "", timeout: 1200 }, ]; -pub fn install_hooks(settings_path: &Path) -> Result<(), ActualError> { +pub fn install_hooks(settings_path: &Path, localhost: bool, hook_all: bool) -> Result<(), ActualError> { let mut settings: Value = if settings_path.exists() { let content = fs::read_to_string(settings_path).map_err(|e| { ActualError::ConfigError(format!("failed to read {}: {e}", settings_path.display())) @@ -48,7 +52,22 @@ pub fn install_hooks(settings_path: &Path) -> Result<(), ActualError> { ActualError::ConfigError("hooks is not a JSON object".to_string()) })?; - for entry in HOOK_ENTRIES { + let entries: Vec<&HookEntry> = if hook_all { + DEFAULT_HOOK_ENTRIES.iter().chain(EXTRA_HOOK_ENTRIES.iter()).collect() + } else { + DEFAULT_HOOK_ENTRIES.iter().collect() + }; + + for entry in entries { + let command = if localhost { + format!( + "ACTUAL_AUTH_URL=http://localhost:3000 ACTUAL_API_URL=http://localhost:3002 {}", + entry.command + ) + } else { + entry.command.to_string() + }; + let entries = hooks_obj .entry(entry.hook_name) .or_insert_with(|| serde_json::json!([])); @@ -57,7 +76,7 @@ pub fn install_hooks(settings_path: &Path) -> Result<(), ActualError> { ActualError::ConfigError(format!("hooks.{} is not an array", entry.hook_name)) })?; - let already_present = arr.iter().any(|matcher_group| { + let existing_idx = arr.iter().position(|matcher_group| { let matcher_matches = matcher_group .get("matcher") .and_then(|m| m.as_str()) @@ -67,24 +86,39 @@ pub fn install_hooks(settings_path: &Path) -> Result<(), ActualError> { .and_then(|h| h.as_array()) .map(|hooks| { hooks.iter().any(|hook| { - hook.get("command").and_then(|c| c.as_str()) == Some(entry.command) + let cmd = hook.get("command").and_then(|c| c.as_str()).unwrap_or(""); + cmd == command || cmd.ends_with(entry.command) }) }) .unwrap_or(false); matcher_matches && command_matches }); - if !already_present { - arr.push(serde_json::json!({ - "matcher": entry.matcher, - "hooks": [ - { - "type": "command", - "command": entry.command, - "timeout": entry.timeout - } - ] - })); + match existing_idx { + Some(idx) => { + arr[idx] = serde_json::json!({ + "matcher": entry.matcher, + "hooks": [ + { + "type": "command", + "command": command, + "timeout": entry.timeout + } + ] + }); + } + None => { + arr.push(serde_json::json!({ + "matcher": entry.matcher, + "hooks": [ + { + "type": "command", + "command": command, + "timeout": entry.timeout + } + ] + })); + } } } @@ -121,32 +155,54 @@ mod tests { use tempfile::tempdir; #[test] - fn test_inserts_hooks_into_empty_settings() { + fn test_inserts_default_hooks_into_empty_settings() { let dir = tempdir().unwrap(); let path = dir.path().join("settings.json"); - install_hooks(&path).unwrap(); + install_hooks(&path, false, false).unwrap(); let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); let hooks = content["hooks"].as_object().unwrap(); - assert_eq!(hooks.len(), 9); + assert_eq!(hooks.len(), 5, "default installs 5 hook types: SessionStart, UserPromptSubmit, PreToolUse, Stop, SessionEnd"); + let expected_default = vec!["SessionStart", "UserPromptSubmit", "PreToolUse", "Stop", "SessionEnd"]; + for hook in &expected_default { + assert!(hooks.contains_key(*hook), "missing default hook: {hook}"); + } assert_eq!( hooks["SessionStart"][0]["hooks"][0]["command"].as_str().unwrap(), "actual observe session-start" ); - // PreToolUse now has 3 matcher-specific entries let pre_tool = hooks["PreToolUse"].as_array().unwrap(); - assert_eq!(pre_tool.len(), 3); - assert_eq!(pre_tool[0]["matcher"].as_str().unwrap(), "Edit|Write"); - assert_eq!(pre_tool[0]["hooks"][0]["timeout"].as_u64().unwrap(), 30); - assert_eq!(pre_tool[1]["matcher"].as_str().unwrap(), "Bash"); - assert_eq!(pre_tool[2]["matcher"].as_str().unwrap(), "Agent"); - assert_eq!(pre_tool[2]["hooks"][0]["timeout"].as_u64().unwrap(), 600); - assert_eq!( - hooks["SessionStart"][0]["matcher"].as_str().unwrap(), - "" - ); + assert_eq!(pre_tool.len(), 1, "default has only ExitPlanMode matcher"); + assert_eq!(pre_tool[0]["matcher"].as_str().unwrap(), "ExitPlanMode"); + } + + #[test] + fn test_hook_all_installs_all_hooks() { + let dir = tempdir().unwrap(); + let path = dir.path().join("settings.json"); + + install_hooks(&path, false, true).unwrap(); + + let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); + let hooks = content["hooks"].as_object().unwrap(); + + let expected = vec![ + "SessionStart", "UserPromptSubmit", "PreToolUse", + "PostToolUse", "PostToolUseFailure", "Stop", + "SessionEnd", "PreCompact", "SubagentStart", + ]; + for hook in &expected { + assert!(hooks.contains_key(*hook), "missing hook: {hook}"); + } + let pre_tool = hooks["PreToolUse"].as_array().unwrap(); + assert_eq!(pre_tool.len(), 4, "hook-all has 4 PreToolUse matchers: ExitPlanMode, Edit|Write, Bash, Agent"); + assert_eq!(pre_tool[0]["matcher"].as_str().unwrap(), "ExitPlanMode"); + assert_eq!(pre_tool[1]["matcher"].as_str().unwrap(), "Edit|Write"); + assert_eq!(pre_tool[2]["matcher"].as_str().unwrap(), "Bash"); + assert_eq!(pre_tool[3]["matcher"].as_str().unwrap(), "Agent"); + assert_eq!(pre_tool[3]["hooks"][0]["timeout"].as_u64().unwrap(), 600); } #[test] @@ -163,7 +219,7 @@ mod tests { }); fs::write(&path, serde_json::to_string(&existing).unwrap()).unwrap(); - install_hooks(&path).unwrap(); + install_hooks(&path, false, false).unwrap(); let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); let session_start = content["hooks"]["SessionStart"].as_array().unwrap(); @@ -180,26 +236,33 @@ mod tests { } #[test] - fn test_idempotent() { + fn test_idempotent_default() { let dir = tempdir().unwrap(); let path = dir.path().join("settings.json"); - install_hooks(&path).unwrap(); - install_hooks(&path).unwrap(); + install_hooks(&path, false, false).unwrap(); + install_hooks(&path, false, false).unwrap(); let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); let session_start = content["hooks"]["SessionStart"].as_array().unwrap(); - assert_eq!( - session_start.len(), - 1, - "should not duplicate hooks on re-run" - ); + assert_eq!(session_start.len(), 1, "should not duplicate hooks on re-run"); let pre_tool = content["hooks"]["PreToolUse"].as_array().unwrap(); - assert_eq!( - pre_tool.len(), - 3, - "PreToolUse should have exactly 3 matcher entries after re-run" - ); + assert_eq!(pre_tool.len(), 1, "default PreToolUse should have exactly 1 matcher entry after re-run"); + } + + #[test] + fn test_idempotent_hook_all() { + let dir = tempdir().unwrap(); + let path = dir.path().join("settings.json"); + + install_hooks(&path, false, true).unwrap(); + install_hooks(&path, false, true).unwrap(); + + let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); + let session_start = content["hooks"]["SessionStart"].as_array().unwrap(); + assert_eq!(session_start.len(), 1, "should not duplicate hooks on re-run"); + let pre_tool = content["hooks"]["PreToolUse"].as_array().unwrap(); + assert_eq!(pre_tool.len(), 4, "hook-all PreToolUse should have exactly 4 matcher entries after re-run"); } #[test] @@ -213,7 +276,7 @@ mod tests { }); fs::write(&path, serde_json::to_string(&existing).unwrap()).unwrap(); - install_hooks(&path).unwrap(); + install_hooks(&path, false, false).unwrap(); let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); assert_eq!(content["model"].as_str().unwrap(), "opus"); @@ -226,46 +289,17 @@ mod tests { let dir = tempdir().unwrap(); let path = dir.path().join("nested").join("deep").join("settings.json"); - install_hooks(&path).unwrap(); + install_hooks(&path, false, false).unwrap(); assert!(path.exists()); } - #[test] - fn test_all_eight_hooks_installed() { - let dir = tempdir().unwrap(); - let path = dir.path().join("settings.json"); - - install_hooks(&path).unwrap(); - - let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); - let hooks = content["hooks"].as_object().unwrap(); - - let expected = vec![ - "SessionStart", - "UserPromptSubmit", - "PreToolUse", - "PostToolUse", - "PostToolUseFailure", - "Stop", - "SessionEnd", - "PreCompact", - "SubagentStart", - ]; - for hook in &expected { - assert!( - hooks.contains_key(*hook), - "missing hook: {hook}" - ); - } - } - #[test] fn test_hook_entries_have_correct_type_and_valid_timeout() { let dir = tempdir().unwrap(); let path = dir.path().join("settings.json"); - install_hooks(&path).unwrap(); + install_hooks(&path, false, true).unwrap(); let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); let hooks = content["hooks"].as_object().unwrap(); @@ -283,4 +317,64 @@ mod tests { } } } + + #[test] + fn test_localhost_flag_prefixes_commands() { + let dir = tempdir().unwrap(); + let path = dir.path().join("settings.json"); + + install_hooks(&path, true, false).unwrap(); + + let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); + let hooks = content["hooks"].as_object().unwrap(); + + let prefix = "ACTUAL_AUTH_URL=http://localhost:3000 ACTUAL_API_URL=http://localhost:3002 "; + assert_eq!( + hooks["SessionStart"][0]["hooks"][0]["command"].as_str().unwrap(), + format!("{}actual observe session-start", prefix) + ); + assert_eq!( + hooks["PreToolUse"][0]["hooks"][0]["command"].as_str().unwrap(), + format!("{}actual observe pre-tool", prefix) + ); + assert_eq!( + hooks["Stop"][0]["hooks"][0]["command"].as_str().unwrap(), + format!("{}actual observe stop", prefix) + ); + } + + #[test] + fn test_localhost_to_production_replaces_commands() { + let dir = tempdir().unwrap(); + let path = dir.path().join("settings.json"); + + install_hooks(&path, true, false).unwrap(); + install_hooks(&path, false, false).unwrap(); + + let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); + let session_start = content["hooks"]["SessionStart"].as_array().unwrap(); + assert_eq!(session_start.len(), 1); + assert_eq!( + session_start[0]["hooks"][0]["command"].as_str().unwrap(), + "actual observe session-start" + ); + } + + #[test] + fn test_production_to_localhost_replaces_commands() { + let dir = tempdir().unwrap(); + let path = dir.path().join("settings.json"); + + install_hooks(&path, false, false).unwrap(); + install_hooks(&path, true, false).unwrap(); + + let content: Value = serde_json::from_str(&fs::read_to_string(&path).unwrap()).unwrap(); + let session_start = content["hooks"]["SessionStart"].as_array().unwrap(); + assert_eq!(session_start.len(), 1); + let prefix = "ACTUAL_AUTH_URL=http://localhost:3000 ACTUAL_API_URL=http://localhost:3002 "; + assert_eq!( + session_start[0]["hooks"][0]["command"].as_str().unwrap(), + format!("{}actual observe session-start", prefix) + ); + } }