examples/record-replay: fix commands broken by the safe-defaults sweep (#133)

Author: kvey

Cargo.toml

@@ -4,6 +4,12 @@ members = [
     "crates/chidori-js",
     "crates/test262-runner",
 ]
+# Bare `cargo run` / `cargo build` at the workspace root mean the CLI — the
+# form every README and doc uses (`cargo run -- run agents/my_agent.ts`).
+# Without this, the test262-runner binary makes `cargo run` ambiguous and it
+# refuses to run anything. CI's workspace-wide jobs pass `--workspace`
+# explicitly, so they are unaffected.
+default-members = ["crates/chidori"]
 resolver = "2"
 
 # Workspace-wide lint levels, inherited by every crate via `[lints]

crates/chidori/src/main.rs

@@ -1253,10 +1253,14 @@ fn cmd_run_stream(
             Ok(())
         }
         Err(e) => {
+            // Frames arrive in transpiled coordinates; the stream consumer
+            // sees the same original-TypeScript positions the CLI reporter
+            // shows. The returned error stays raw — report_cli_error remaps
+            // it once at its own display boundary.
             let line = serde_json::json!({
                 "type": "done",
                 "status": "failed",
-                "error": format!("{e:#}"),
+                "error": crate::runtime::rust_engine::remap_stack_frames(&format!("{e:#}")),
             });
             println!("{line}");
             Err(e)

crates/chidori/src/runtime/rust_engine.rs

@@ -214,9 +214,21 @@ pub(crate) fn set_display_project_root(root: PathBuf) {
 pub(crate) fn read_project_source(file: &str) -> Option<String> {
     let root = DISPLAY_PROJECT_ROOT
         .with(|r| r.borrow().clone())
-        .or_else(|| std::env::current_dir().ok())?
-        .canonicalize()
-        .ok()?;
+        .or_else(|| std::env::current_dir().ok())?;
+    read_project_source_within(&root, file)
+}
+
+/// As [`read_project_source`], but confined to an explicit `root` instead of
+/// the thread-local display root — for surfaces whose display boundary is not
+/// the CLI process's JS thread (the HTTP server's session errors). An empty
+/// root (the parent of a bare `agent.ts`) means the current directory.
+pub(crate) fn read_project_source_within(root: &Path, file: &str) -> Option<String> {
+    let root = if root.as_os_str().is_empty() {
+        Path::new(".")
+    } else {
+        root
+    };
+    let root = root.canonicalize().ok()?;
     let full = Path::new(file).canonicalize().ok()?;
     full.starts_with(&root)
         .then(|| std::fs::read_to_string(&full).ok())
@@ -283,6 +295,19 @@ pub(crate) fn parse_stack_frame(line: &str) -> Option<StackFrame<'_>> {
 /// uniform transpiled coordinates); error path only, so each distinct file
 /// re-runs the transpile pipeline once with map generation on.
 pub(crate) fn remap_stack_frames(err: &str) -> String {
+    remap_stack_frames_via(err, read_project_source)
+}
+
+/// As [`remap_stack_frames`], but confining frame source reads to an explicit
+/// project root — the server's variant, applied where a run error becomes a
+/// session's stored/returned `error` (the CLI resolves its root from the
+/// thread-local set at command startup; the server handlers run on tokio
+/// threads that never set it, so the agent's workspace root is passed in).
+pub(crate) fn remap_stack_frames_within(root: &Path, err: &str) -> String {
+    remap_stack_frames_via(err, |file| read_project_source_within(root, file))
+}
+
+fn remap_stack_frames_via(err: &str, read: impl Fn(&str) -> Option<String>) -> String {
     use std::collections::HashMap;
     let mut sources: HashMap<&str, Option<String>> = HashMap::new();
     let mut out = String::with_capacity(err.len());
@@ -294,7 +319,7 @@ pub(crate) fn remap_stack_frames(err: &str) -> String {
             let file = frame.file?;
             let source = sources
                 .entry(file)
-                .or_insert_with(|| read_project_source(file))
+                .or_insert_with(|| read(file))
                 .as_deref()?;
             let pos = crate::runtime::typescript::transpile::remap_to_original(
                 Path::new(file),

crates/chidori/src/server.rs

@@ -676,7 +676,8 @@ pub async fn serve(
     let acp_state = AcpState {
         store: state.session_store.clone(),
         run_prompt: Arc::new(move |inputs: Value| -> Result<Value, String> {
-            run_agent_sync(&acp_runner_state, inputs).map_err(|e| e.to_string())
+            run_agent_sync(&acp_runner_state, inputs)
+                .map_err(|e| agent_error_string(&acp_runner_state.agent_path, &e))
         }),
     };
 
@@ -1350,7 +1351,7 @@ async fn create_session(
     };
     match result {
         Ok(run_result) => apply_run_outcome(&mut session, run_result),
-        Err(e) => session.error = Some(e.to_string()),
+        Err(e) => session.error = Some(agent_error_string(&state.agent_path, &e)),
     }
 
     if let Some(err) = store_or_500(&state, &session) {
@@ -1911,7 +1912,8 @@ async fn stream_session(
                         Err(e) => {
                             session.status = SessionStatus::Failed;
                             session.output = None;
-                            session.error = Some(e.to_string());
+                            session.error =
+                                Some(agent_error_string(&state_for_stream.agent_path, &e));
                         }
                     }
                     if was_cancelled {
@@ -2107,6 +2109,18 @@ async fn cancel_session(
     .into_response()
 }
 
+/// Render an agent-run error for a session's stored/returned `error` field.
+/// Uncaught-exception stack frames arrive from the engine in transpiled
+/// coordinates; remap them to positions in the original TypeScript against
+/// the served agent's workspace root — the same display-boundary remap the
+/// CLI applies in `main::report_cli_error` (which resolves its root from a
+/// thread-local these tokio handlers never set). Frames that can't be read
+/// or remapped pass through unchanged, as does any error without frames.
+fn agent_error_string(agent_path: &FsPath, e: &anyhow::Error) -> String {
+    let root = crate::runtime::typescript::transpile::find_workspace_root(agent_path);
+    crate::runtime::rust_engine::remap_stack_frames_within(&root, &e.to_string())
+}
+
 /// Map a finished engine run onto a stored session: status, output, call log,
 /// and the pending-pause fields (input prompt / signal listen set + timeout
 /// deadline / approval). Shared by session creation, the durable resume tail,
@@ -2363,13 +2377,14 @@ async fn complete_pending_and_resume(
             (StatusCode::OK, Json(session_view(&session))).into_response()
         }
         Err(e) => {
+            let error = agent_error_string(&state.agent_path, &e);
             session.status = SessionStatus::Failed;
-            session.error = Some(e.to_string());
+            session.error = Some(error.clone());
             let _ = state.session_store.put(&session);
             state.warm_runs.lock().unwrap().remove(&session.id);
             (
                 StatusCode::INTERNAL_SERVER_ERROR,
-                Json(json!({"error": e.to_string()})),
+                Json(json!({"error": error})),
             )
                 .into_response()
         }
@@ -2447,7 +2462,7 @@ async fn resume_session(
                     Some(Ok(run_result)) => apply_run_outcome(&mut session, run_result),
                     Some(Err(e)) => {
                         session.status = SessionStatus::Failed;
-                        session.error = Some(e.to_string());
+                        session.error = Some(agent_error_string(&state.agent_path, &e));
                     }
                     None => {
                         session.status = SessionStatus::Failed;
@@ -2927,12 +2942,13 @@ async fn approve_session(
             (StatusCode::OK, Json(session_view(&session))).into_response()
         }
         Err(e) => {
+            let error = agent_error_string(&state.agent_path, &e);
             session.status = SessionStatus::Failed;
-            session.error = Some(e.to_string());
+            session.error = Some(error.clone());
             let _ = state.session_store.put(&session);
             (
                 StatusCode::INTERNAL_SERVER_ERROR,
-                Json(json!({"error": e.to_string()})),
+                Json(json!({"error": error})),
             )
                 .into_response()
         }
@@ -3012,7 +3028,7 @@ async fn handle_event(
         }
         Err(e) => {
             eprintln!("Agent error: {e:#}");
-            let error = json!({"error": e.to_string()});
+            let error = json!({"error": agent_error_string(&state.agent_path, &e)});
             (StatusCode::INTERNAL_SERVER_ERROR, Json(error)).into_response()
         }
     }
@@ -3026,6 +3042,73 @@ mod tests {
     };
     use axum::body;
 
+    /// A failed session's `error` must carry stack frames in ORIGINAL
+    /// TypeScript coordinates for every frame, not just the throwing one —
+    /// the engine hands the server transpiled-bundle positions, and the
+    /// server (whose tokio handlers never set the CLI's thread-local display
+    /// root) remaps them against the agent's workspace root.
+    #[test]
+    fn agent_error_string_remaps_every_frame_to_original_source() {
+        let dir = std::env::temp_dir().join(format!("chidori-srv-frames-{}", uuid::Uuid::new_v4()));
+        std::fs::create_dir_all(&dir).unwrap();
+        let agent_path = dir.join("agent.ts");
+        // The interface block exists only in the original TypeScript, so every
+        // transpiled line number below it disagrees with the original.
+        let src = "interface Row {\n\
+                   \x20 id: string;\n\
+                   }\n\
+                   function inner(row: Row): never {\n\
+                   \x20 throw new Error(\"bad \" + row.id);\n\
+                   }\n\
+                   function outer(): never {\n\
+                   \x20 return inner({ id: \"x\" });\n\
+                   }\n\
+                   export async function agent() { return outer(); }\n";
+        std::fs::write(&agent_path, src).unwrap();
+
+        // Derive the frames' transpiled coordinates the same way the engine
+        // stamps them: the emitted definition line of each function.
+        let (js, _map) =
+            crate::runtime::typescript::transpile::transpile_source_with_map(&agent_path, src)
+                .unwrap();
+        let emitted_line = |name: &str| {
+            js.lines()
+                .position(|l| l.contains(&format!("function {name}")))
+                .map(|i| i as u32 + 1)
+                .unwrap()
+        };
+        let (inner_line, outer_line) = (emitted_line("inner"), emitted_line("outer"));
+        // The interface strip is what makes this test meaningful: emitted
+        // positions must disagree with the original definition lines (4, 7).
+        assert_ne!(
+            inner_line, 4,
+            "transpile no longer shifts lines; rewrite this test"
+        );
+
+        let path_str = agent_path.to_string_lossy();
+        let err = anyhow::anyhow!(
+            "JavaScript exception: Error: bad x\n    at inner ({path_str}:{inner_line}:10)\n    at outer ({path_str}:{outer_line}:10)"
+        );
+        let remapped = agent_error_string(&agent_path, &err);
+        assert!(
+            remapped.contains(&format!("at inner ({path_str}:4:")),
+            "throwing frame lands on the original definition line: {remapped}"
+        );
+        assert!(
+            remapped.contains(&format!("at outer ({path_str}:7:")),
+            "the frame ABOVE the throwing one lands on its original line too: {remapped}"
+        );
+
+        // Errors without frames pass through byte-identical.
+        let plain = anyhow::anyhow!("policy: `tool:x` denied");
+        assert_eq!(
+            agent_error_string(&agent_path, &plain),
+            "policy: `tool:x` denied"
+        );
+
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
     #[test]
     fn bearer_token_matches_single_key() {
         assert!(bearer_token_matches("Bearer sekrit", "sekrit"));

examples/interactive-pipeline/README.md

@@ -35,13 +35,18 @@ examples/interactive-pipeline/run.sh
 
 # …or directly
 chidori run examples/interactive-pipeline/interactive_pipeline.ts \
-  -i '{"pipeline":"triage","stages":5,"itemsPerStage":4}'
+  -i '{"pipeline":"triage","stages":5,"itemsPerStage":4}' --trusted
 
 # …or from source
 cargo run -- run examples/interactive-pipeline/interactive_pipeline.ts \
-  -i '{"pipeline":"triage","stages":5,"itemsPerStage":4}'
+  -i '{"pipeline":"triage","stages":5,"itemsPerStage":4}' --trusted
 ```
 
+(`--trusted`: the per-stage `review_batch` tool call is a gated effect, and
+`chidori run` is ask-by-default — without the flag each stage stops for an
+extra y/N approval before its checkpoint. This is in-repo code you're running
+on yourself; see [`docs/running-modes.md`](../../docs/running-modes.md).)
+
 At each checkpoint the agent prints a prompt to your terminal and **blocks on
 stdin** (`chidori.input`). Type one of:
 
@@ -62,7 +67,7 @@ export OTEL_EXPORTER_OTLP_ENDPOINT=http://127.0.0.1:4317
 export OTEL_SERVICE_NAME=interactive-pipeline        # optional (defaults to "chidori")
 
 chidori run examples/interactive-pipeline/interactive_pipeline.ts \
-  -i '{"pipeline":"triage","stages":5,"itemsPerStage":4}'
+  -i '{"pipeline":"triage","stages":5,"itemsPerStage":4}' --trusted
 ```
 
 `run.sh` does this for you, but only when tael is actually listening on `:4317`.

examples/interactive-pipeline/run.sh

@@ -29,5 +29,8 @@ else
   echo "  (start tael, then re-run; or set OTEL_EXPORTER_OTLP_ENDPOINT yourself)" >&2
 fi
 
+# --trusted: the per-stage review_batch tool call is a gated effect, and
+# `chidori run` asks before gated effects by default — the flag keeps the
+# session's only prompts the agent's own checkpoints.
 exec cargo run --quiet --manifest-path "$REPO/Cargo.toml" \
-  -- run "$AGENT" -i "$INPUT"
+  -- run "$AGENT" -i "$INPUT" --trusted