feat(sidecar,shell): 24-step turns, failure output reaches the model (v0.5.1 HGW-W5/W6)

agent-shell/src/lib.rs

@@ -28,7 +28,8 @@
 //!   `KILL_ON_JOB_CLOSE`); see the Windows section below.
 //! - **Capped capture.** stdout and stderr are captured separately up to per-stream byte caps,
 //!   with a truncation marker and the true byte count, and drained to EOF so a chatty child
-//!   never blocks on a full pipe.
+//!   never blocks on a full pipe. HGW-4: a capped stream keeps its first half-cap and its last
+//!   half-cap of bytes (the first error and the final cause), with the marker between them.
 //! - **A [`RunReport`]** with exit code / signal / timeout / duration that serializes to JSON
 //!   for the S1.3 verifiers (`JsonFieldEquals` on `exit_code`, `passed`, ...).
 //!
@@ -1570,9 +1571,12 @@ fn wait_with_deadline(child: &mut Child, tree: &ProcessTree, deadline: Instant)
 // Output capture
 // ------------------------------------------------------------------------------------------
 
+/// HGW-4: the first half of the cap is kept from the start of the stream and the second half
+/// from its end, so a long failing build keeps both its first error and its final cause.
 #[derive(Default)]
 struct CaptureState {
-    kept: Vec<u8>,
+    head: Vec<u8>,
+    tail: std::collections::VecDeque<u8>,
     total: u64,
 }
 
@@ -1604,9 +1608,16 @@ impl Capture {
                     Ok(n) => {
                         let mut s = shared.lock().unwrap_or_else(|p| p.into_inner());
                         s.total += n as u64;
-                        let room = cap.saturating_sub(s.kept.len());
+                        let head_cap = cap.div_ceil(2);
+                        let tail_cap = cap - head_cap;
+                        let room = head_cap.saturating_sub(s.head.len());
                         let take = room.min(n);
-                        s.kept.extend_from_slice(&buf[..take]);
+                        s.head.extend_from_slice(&buf[..take]);
+                        if tail_cap > 0 {
+                            s.tail.extend(&buf[take..n]);
+                            let excess = s.tail.len().saturating_sub(tail_cap);
+                            s.tail.drain(..excess);
+                        }
                     }
                     Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue,
                     Err(_) => break,
@@ -1629,15 +1640,24 @@ impl Capture {
                 .is_ok(),
         };
         let s = self.state.lock().unwrap_or_else(|p| p.into_inner());
-        let truncated = s.total > s.kept.len() as u64;
+        let tail: Vec<u8> = s.tail.iter().copied().collect();
+        let kept = (s.head.len() + tail.len()) as u64;
+        let truncated = s.total > kept;
         let text = if truncated {
-            let omitted = s.total.saturating_sub(s.kept.len() as u64);
-            format!(
+            let omitted = s.total.saturating_sub(kept);
+            let mut t = format!(
                 "{}\n[truncated: {omitted} bytes omitted]",
-                lossy_prefix(&s.kept)
-            )
+                lossy_prefix(&s.head)
+            );
+            if !tail.is_empty() {
+                t.push('\n');
+                t.push_str(&lossy_suffix(&tail));
+            }
+            t
         } else {
-            String::from_utf8_lossy(&s.kept).into_owned()
+            let mut all = s.head.clone();
+            all.extend_from_slice(&tail);
+            String::from_utf8_lossy(&all).into_owned()
         };
         Captured {
             text,
@@ -1671,6 +1691,17 @@ fn lossy_prefix(bytes: &[u8]) -> String {
     String::from_utf8_lossy(&bytes[..keep]).into_owned()
 }
 
+/// Lossy UTF-8 of a capped suffix: continuation bytes of a character cut in half at the start
+/// are dropped rather than rendered as a replacement character.
+fn lossy_suffix(bytes: &[u8]) -> String {
+    let skip = bytes
+        .iter()
+        .take(3)
+        .take_while(|b| (**b & 0xC0) == 0x80)
+        .count();
+    String::from_utf8_lossy(&bytes[skip..]).into_owned()
+}
+
 // ------------------------------------------------------------------------------------------
 /// Create `home/rel` as a symlink to `target` (a copy where symlinks are not available).
 fn link_into(home: &Path, rel: &Path, target: &Path) -> std::io::Result<()> {

agent-shell/tests/runner.rs

@@ -384,9 +384,43 @@ fn stdout_is_capped_with_a_truncation_marker() {
         "{}",
         &r.stdout[900..]
     );
+    // HGW-4: the end of the output is kept too (a failure's cause is usually last).
+    assert!(r.stdout.ends_with("19999\n20000\n"), "{}", &r.stdout[900..]);
     assert!(r.stdout.len() < 1100);
 }
 
+/// HGW-4: a long, failing build keeps both its first lines (where compilers report the first
+/// error) and its last lines (where npm, cargo and forge print the cause), with the middle cut.
+#[test]
+fn capped_output_keeps_the_head_and_the_tail() {
+    let d = tmp();
+    let policy = ShellPolicy::new(test_allowlist(), sys_path())
+        .expect("policy")
+        .with_output_caps(4096, 4096);
+    let r = open_runner(policy)
+        .run(&req(
+            "sh",
+            &[
+                "-c",
+                "echo FIRST-ERROR >&2; seq 1 50000 >&2; echo 'npm ERR! missing script: build' >&2; exit 1",
+            ],
+            d.path(),
+        ))
+        .expect("run");
+    assert_eq!(r.exit_code, Some(1));
+    assert!(r.stderr_truncated);
+    assert!(r.stderr.starts_with("FIRST-ERROR\n"), "{}", &r.stderr[..64]);
+    assert!(
+        r.stderr
+            .trim_end()
+            .ends_with("npm ERR! missing script: build"),
+        "{}",
+        &r.stderr[r.stderr.len() - 64..]
+    );
+    assert!(r.stderr.contains("bytes omitted]"));
+    assert!(r.stderr.len() <= 4096 + 64, "{}", r.stderr.len());
+}
+
 #[test]
 fn stderr_is_captured_separately_and_capped() {
     let d = tmp();
@@ -404,7 +438,9 @@ fn stderr_is_captured_separately_and_capped() {
     assert!(!r.passed());
     assert_eq!(r.stdout, "out\n");
     assert!(r.stderr_truncated);
-    assert!(r.stderr.starts_with("0123456789"));
+    // HGW-4: half the cap from the start, half from the end.
+    assert!(r.stderr.starts_with("01234"), "{}", r.stderr);
+    assert!(r.stderr.ends_with("cdef\n"), "{}", r.stderr);
     assert_eq!(r.stderr_bytes, 17);
 }
 

agent-sidecar/src/lib.rs

@@ -1860,6 +1860,10 @@ pub fn production_sessions_with(
     let mgr = mgr.with_self_review(self_review_from_env_var(
         std::env::var("CITRATE_HERMES_SELF_REVIEW").ok().as_deref(),
     ));
+    // HGW-4: turns run up to 24 model calls unless CITRATE_HERMES_MAX_STEPS says otherwise.
+    let mgr = mgr.with_default_max_steps(sessions::max_steps_from_env(
+        std::env::var("CITRATE_HERMES_MAX_STEPS").ok().as_deref(),
+    ));
     // HGW-3: project_create (off by default; every project needs the member's approval).
     let mgr = match projects::ProjectsConfig::from_env_vars(
         |k| std::env::var(k).ok(),
@@ -2383,6 +2387,8 @@ mod deploy_guard_session_tests;
 #[cfg(all(test, unix))]
 mod hgw_support;
 #[cfg(all(test, unix))]
+mod loop_capability_tests;
+#[cfg(all(test, unix))]
 mod project_policy_tests;
 #[cfg(all(test, unix))]
 mod projects_tests;

agent-sidecar/src/loop_capability_tests.rs

@@ -0,0 +1,90 @@
+//! HGW-4 — the loop can finish a build: more steps per turn by default (8 → 24, configurable),
+//! and a failing command or toolchain run hands the model its exit status and the end of its
+//! output (bounded), so it can read the cause and fix it.
+//!
+//! BDD map:
+//! - `a_session_runs_up_to_24_steps_by_default`
+//! - `max_steps_is_configurable_per_session_and_per_sidecar`
+//! - `the_sidecar_default_comes_from_the_environment` (CITRATE_HERMES_MAX_STEPS)
+//! - shell_run and toolchain failures: `shell_run_tests::a_failing_command_reports_*`,
+//!   `toolchain::tests::an_unparsed_report_hands_back_the_end_of_stderr_as_untrusted`.
+
+#![cfg(unix)]
+
+use super::hgw_support::*;
+use super::*;
+use citrate_agent_loop::AssistantTurn;
+
+fn list_calls(fx: &Fx, n: usize) -> Vec<AssistantTurn> {
+    let mut t: Vec<AssistantTurn> = (0..n)
+        .map(|i| {
+            AssistantTurn::tools(vec![tool_call(
+                &format!("l{i}"),
+                grants::FILE_LIST_TOOL,
+                serde_json::json!({ "path": fx.proj() }),
+            )])
+        })
+        .collect();
+    t.push(AssistantTurn::text("Listed."));
+    t
+}
+
+async fn outcome(mgr: Arc<sessions::SessionManager>, req: sessions::CreateSessionReq) -> String {
+    let id = mgr.create(req).expect("session");
+    let s = mgr.get(&id).expect("session");
+    mgr.send(&id, "list it a lot".into(), None).expect("send");
+    wait_done(&s).await;
+    events(&s)
+        .into_iter()
+        .rev()
+        .find(|e| e["type"] == "done")
+        .map(|e| e["outcome"].as_str().unwrap_or_default().to_string())
+        .unwrap_or_default()
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn a_session_runs_up_to_24_steps_by_default() {
+    let fx = Fx::new("steps-default");
+    // 20 tool steps and an answer: more than the old default of 8.
+    let mgr = Arc::new(manager_with(&fx, list_calls(&fx, 20), None));
+    assert_eq!(
+        outcome(mgr, create_req(fx.rw_doc(), true)).await,
+        "answered"
+    );
+    assert_eq!(sessions::DEFAULT_MAX_STEPS, 24);
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn max_steps_is_configurable_per_session_and_per_sidecar() {
+    let fx = Fx::new("steps-config");
+    let mgr = Arc::new(manager_with(&fx, list_calls(&fx, 20), None));
+    let req = create_req_with(fx.rw_doc(), true, serde_json::json!({"maxSteps": 4})).expect("req");
+    assert_eq!(outcome(mgr, req).await, "step_limit");
+    // A sidecar-wide default below the work also stops it; a session's own maxSteps wins.
+    let mgr = Arc::new(manager_with(&fx, list_calls(&fx, 10), None).with_default_max_steps(6));
+    assert_eq!(
+        outcome(mgr, create_req(fx.rw_doc(), true)).await,
+        "step_limit"
+    );
+    let mgr = Arc::new(manager_with(&fx, list_calls(&fx, 10), None).with_default_max_steps(6));
+    let req = create_req_with(fx.rw_doc(), true, serde_json::json!({"maxSteps": 12})).expect("req");
+    assert_eq!(outcome(mgr, req).await, "answered");
+}
+
+#[test]
+fn the_sidecar_default_comes_from_the_environment() {
+    assert_eq!(
+        sessions::max_steps_from_env(None),
+        sessions::DEFAULT_MAX_STEPS
+    );
+    assert_eq!(sessions::max_steps_from_env(Some("16")), 16);
+    assert_eq!(
+        sessions::max_steps_from_env(Some("999")),
+        sessions::MAX_STEPS_CAP
+    );
+    assert_eq!(sessions::max_steps_from_env(Some("0")), 1);
+    assert_eq!(
+        sessions::max_steps_from_env(Some("lots")),
+        sessions::DEFAULT_MAX_STEPS
+    );
+}

agent-sidecar/src/main.rs

@@ -30,6 +30,8 @@
 //!                               approves, run in the OS sandbox, never without it) in every
 //!                               session opened with folder grants; anything else = off (optional)
 //!   CITRATE_HERMES_SHELL_PATH   shell_run search path override, a path list (optional)
+//!   CITRATE_HERMES_MAX_STEPS    HGW-4: model calls per turn for sessions that send no maxSteps;
+//!                               default 24, at most 32 (optional)
 //!   CITRATE_HERMES_PROJECTS     HGW-3: `1` offers project_create (a new project folder, after the
 //!                               member approves it) in sessions opened with folder grants (optional)
 //!   CITRATE_HERMES_PROJECTS_ROOT  absolute folder new projects go in; default ~/Citrate/Projects

agent-sidecar/src/sessions.rs

@@ -148,6 +148,18 @@ pub const MAX_SESSIONS: usize = 8;
 pub const EVENT_LOG_CAP: usize = 2000;
 /// Upper bounds a client may request.
 pub const MAX_STEPS_CAP: u32 = 32;
+/// HGW-4: model calls per turn when neither the session (`maxSteps`) nor the sidecar
+/// (`CITRATE_HERMES_MAX_STEPS`) says otherwise. A build (create, write, run, read the error, fix,
+/// run again) takes more than the old default of 8.
+pub const DEFAULT_MAX_STEPS: u32 = 24;
+
+/// HGW-4: the sidecar-wide default for `maxSteps` from `CITRATE_HERMES_MAX_STEPS`: a whole number,
+/// clamped to 1..=[`MAX_STEPS_CAP`]. Unset or not a number = [`DEFAULT_MAX_STEPS`].
+pub fn max_steps_from_env(v: Option<&str>) -> u32 {
+    v.and_then(|s| s.trim().parse::<u64>().ok())
+        .map(|n| n.clamp(1, u64::from(MAX_STEPS_CAP)) as u32)
+        .unwrap_or(DEFAULT_MAX_STEPS)
+}
 pub const MAX_TOKENS_CAP: u32 = 8192;
 /// Longest a long-poll may wait.
 pub const MAX_WAIT_MS: u64 = 25_000;
@@ -1165,6 +1177,8 @@ pub struct SessionManager {
     shell_run: Option<Arc<ShellRunConfig>>,
     /// HGW-3: `project_create` for sessions opened with folder grants (default off).
     projects: Option<Arc<crate::projects::ProjectsConfig>>,
+    /// HGW-4: `maxSteps` for a session that does not send one.
+    default_max_steps: u32,
     /// HUP-S2.6: the one writer of the decision records the nightly anchor batches.
     records: Option<Arc<citrate_agent_records::DecisionLog>>,
     /// US-1.3 AC2: ask the model for a self-review of every workflow step attempt and record it
@@ -1201,6 +1215,7 @@ impl SessionManager {
             uses: AtomicU64::new(0),
             shell_run: None,
             projects: None,
+            default_max_steps: DEFAULT_MAX_STEPS,
             records: None,
             self_review: false,
         }
@@ -1248,6 +1263,12 @@ impl SessionManager {
         self
     }
 
+    /// HGW-4: `maxSteps` for sessions that do not send one (clamped to 1..=[`MAX_STEPS_CAP`]).
+    pub fn with_default_max_steps(mut self, n: u32) -> Self {
+        self.default_max_steps = n.clamp(1, MAX_STEPS_CAP);
+        self
+    }
+
     /// HGW-3: offer `project_create` to every new session opened with folder grants.
     pub fn with_projects(mut self, cfg: Arc<crate::projects::ProjectsConfig>) -> Self {
         self.projects = Some(cfg);
@@ -1807,7 +1828,10 @@ impl SessionManager {
         let cfg = LoopConfig {
             model: req.model,
             system_prompt,
-            max_steps: req.max_steps.unwrap_or(8).clamp(1, MAX_STEPS_CAP),
+            max_steps: req
+                .max_steps
+                .unwrap_or(self.default_max_steps)
+                .clamp(1, MAX_STEPS_CAP),
             max_tool_calls_per_step: req.max_tool_calls_per_step.unwrap_or(4).clamp(1, 16),
             max_tokens: req.max_tokens.unwrap_or(2048).clamp(64, MAX_TOKENS_CAP),
         };

agent-sidecar/src/shell_run.rs

@@ -63,6 +63,10 @@ pub const DEFAULT_TIMEOUT_SECS: u64 = 120;
 pub const MAX_TIMEOUT_SECS: u64 = 600;
 /// Bytes of stdout and of stderr kept from one command.
 pub const OUTPUT_CAP: usize = 32 * 1024;
+/// HGW-4: bytes of a failing command's stderr (or stdout, when stderr is empty) handed to the
+/// model in the result's summary, from the end of the stream, where npm, cargo and tsc print the
+/// cause. The full capture (head and tail) stays in the run facts.
+pub const FAILURE_TAIL_BYTES: usize = 2048;
 /// Most argv entries, longest entry, and most argv bytes a call may propose.
 pub const MAX_ARGV: usize = 64;
 pub const MAX_ARG_BYTES: usize = 4096;
@@ -442,8 +446,20 @@ fn same_command(a: &RunPlan, b: &RunPlan) -> bool {
     a.resolved_path == b.resolved_path && a.args == b.args && a.cwd == b.cwd
 }
 
+/// The last `max` bytes of `s` (on a character boundary).
+pub(crate) fn tail_of(s: &str, max: usize) -> String {
+    if s.len() <= max {
+        return s.to_string();
+    }
+    let mut start = s.len() - max;
+    while !s.is_char_boundary(start) {
+        start += 1;
+    }
+    s[start..].to_string()
+}
+
 fn report(argv: &[String], r: &RunReport) -> ToolOutcome {
-    let facts = serde_json::json!({
+    let mut facts = serde_json::json!({
         "program": r.program,
         "args": r.args,
         "argv": argv,
@@ -509,12 +525,31 @@ fn report(argv: &[String], r: &RunReport) -> ToolOutcome {
         (None, Some(s)) => format!("was ended by signal {s}"),
         (None, None) => "ended without a status".to_string(),
     };
+    // HGW-4: a failing command hands the model the end of its output, so it can fix the cause.
+    let failure_note = if r.exit_code == Some(0) {
+        String::new()
+    } else {
+        let (stream, text) = if r.stderr.trim().is_empty() {
+            ("stdout", r.stdout.as_str())
+        } else {
+            ("stderr", r.stderr.as_str())
+        };
+        let tail = tail_of(text, FAILURE_TAIL_BYTES);
+        if let Some(o) = facts.as_object_mut() {
+            o.insert(format!("{stream}_tail"), serde_json::json!(tail));
+        }
+        if tail.trim().is_empty() {
+            ". It printed nothing".to_string()
+        } else {
+            format!(". The end of its {stream}:\n{tail}")
+        }
+    };
     ToolOutcome::Ok(
         ToolchainEnvelope::not_run(
             SHELL_RUN_TOOL,
             RunStatus::Completed,
             format!(
-                "{} {ended} after {} ms ({isolation}){git_note}",
+                "{} {ended} after {} ms ({isolation}){git_note}{failure_note}",
                 r.program, r.duration_ms
             ),
         )