Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/sdk/src/daemon/mappers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ mod usage;
mod work;
mod workspace;
pub(crate) use workspace::{
pull_request_command, workspace_event_from_output, PendingPullRequestCall,
pull_request_command, workspace_event_from_output, worktree_checkout_from_output,
PendingPullRequestCall,
};

#[cfg(test)]
Expand Down
11 changes: 10 additions & 1 deletion src/sdk/src/daemon/mappers/workspace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub(crate) fn workspace_event_from_output(
ts: i64,
record_type: &str,
) -> Option<HarnessSemanticEvent> {
let checkout = json_report(output).or_else(|| text_report(output));
let checkout = worktree_checkout_from_output(output);
let pull_request = pull_request_command.and_then(|command| match command {
PullRequestCommand::Create => pull_request_url(output),
PullRequestCommand::View => pull_request_url_from_json(output),
Expand Down Expand Up @@ -68,6 +68,15 @@ pub(crate) fn workspace_event_from_output(
))
}

/// Read a stable worktree-helper report from command output.
///
/// Shared with transports that do not use the JSONL mapper (notably Codex
/// app-server), so every Codex execution path applies the same signature checks
/// before changing a session's runtime directory.
pub(crate) fn worktree_checkout_from_output(output: &str) -> Option<(String, String)> {
json_report(output).or_else(|| text_report(output))
}

/// Read the `--json` report, allowing command output around the object.
fn json_report(output: &str) -> Option<(String, String)> {
output.match_indices('{').find_map(|(start, _)| {
Expand Down
2 changes: 1 addition & 1 deletion src/sdk/src/daemon/providers/codex_server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Selected by naming the `codex-server` harness, which reaches here as `HarnessTra

## What this reports

Deliberately minimal: lifecycle status, the assistant's messages, and token usage. The app-server reports far more — per-item reasoning deltas, command output streams, patch previews — and the CLI transport's mappers turn the equivalent into the rich agent-rail detail an operator watches.
Deliberately minimal: lifecycle status, the assistant's messages, token usage, and stable worktree reports that determine where a resumed turn executes. The app-server reports far more — per-item reasoning deltas, command output streams, patch previews — and the CLI transport's mappers turn the equivalent into the rich agent-rail detail an operator watches.

Reproducing that surface here would mean a second implementation of every mapper, tracking a wire format still marked experimental, for a transport chosen when throughput is what matters. So a `codex-server` lane reports that it is working, what it finally said, and what it cost — and an operator who wants to watch a lane work runs it on `codex`.

Expand Down
17 changes: 16 additions & 1 deletion src/sdk/src/daemon/providers/codex_server/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,14 @@ async fn run(options: RunTaskOptions) -> Result<RunTaskResult, AppServerError> {
&connection,
&mut subscription,
&thread_id,
thread.cwd,
options.prompt,
options.model,
options.timeout_ms,
options.abort,
options.on_event,
options.workspace_context,
options.on_workspace_context,
)
.await?;

Expand Down Expand Up @@ -212,16 +215,23 @@ async fn drive_turn(
connection: &Connection,
subscription: &mut ThreadSubscription,
thread_id: &str,
cwd: String,
prompt: String,
model: Option<String>,
timeout_ms: u64,
abort: super::super::types::Abort,
on_event: Option<OnEvent>,
workspace_context: crate::sessions::WorkspaceContext,
on_workspace_context: Option<super::super::types::OnWorkspaceContext>,
) -> Result<TurnOutcome, AppServerError> {
// Shared because the idle watchdog reads the fold's last-activity stamp
// while the notification branch writes to it, and both live in one
// `select!`.
let fold = Arc::new(Mutex::new(FoldState::new(on_event)));
let fold = Arc::new(Mutex::new(FoldState::with_workspace(
on_event,
workspace_context,
on_workspace_context,
)));
let timeout = Duration::from_millis(timeout_ms);
// Tracked from `turn/started` so an interrupt can name the turn it stops;
// the protocol requires both ids.
Expand All @@ -231,6 +241,11 @@ async fn drive_turn(
"turn/start",
json!({
"threadId": thread_id,
// Codex treats cwd as sticky turn state. Send it even after
// `thread/resume`: otherwise a retained worktree exists only in
// Medulla's metadata while built-in tools default to the original
// directory recorded by the thread.
"cwd": cwd,
"input": [{ "type": "text", "text": prompt }],
"model": model,
}),
Expand Down
66 changes: 60 additions & 6 deletions src/sdk/src/daemon/providers/codex_server/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
//!
//! # Scope
//!
//! Deliberately minimal: lifecycle status, the assistant's messages, and token
//! usage. The app-server reports far more than that — per-item reasoning deltas,
//! command output streams, patch previews — and the CLI transport's mappers turn
//! the equivalent into the rich agent-rail detail an operator watches.
//! Deliberately minimal: lifecycle status, the assistant's messages, token
//! usage, and repository moves that affect where the next turn executes. The
//! app-server reports far more than that — per-item reasoning deltas, command
//! output streams, patch previews — and the CLI transport's mappers turn the
//! equivalent into the rich agent-rail detail an operator watches.
//!
//! Reproducing that surface here would mean a second implementation of every
//! mapper, tracking a wire format that is still marked experimental, for a
Expand All @@ -22,10 +23,11 @@ use std::time::Instant;
use serde_json::{json, Value};

use crate::codex_app_server::Notification;
use crate::daemon::mappers::HarnessSemanticEvent;
use crate::daemon::mappers::{worktree_checkout_from_output, HarnessSemanticEvent};
use crate::protocol::{HarnessEvent, TokenUsage};
use crate::sessions::WorkspaceContext;

use super::super::types::OnEvent;
use super::super::types::{OnEvent, OnWorkspaceContext};

/// What a finished fold reports, without the callback it folded through.
#[derive(Debug, Clone, Default)]
Expand Down Expand Up @@ -58,6 +60,10 @@ pub(super) struct FoldState {
pub(super) last_activity: Instant,
/// Per-event status callback.
on_event: Option<OnEvent>,
/// Repository position retained across turns of this thread.
workspace_context: WorkspaceContext,
/// Persists a newly detected worktree for the next resumed turn.
on_workspace_context: Option<OnWorkspaceContext>,
/// Line counter standing in for the CLI transport's transcript offsets.
///
/// There is no transcript here, but `HarnessSemanticEvent::line` is the
Expand All @@ -68,14 +74,26 @@ pub(super) struct FoldState {

impl FoldState {
/// A fold ready for one turn.
#[cfg(test)]
pub(super) fn new(on_event: Option<OnEvent>) -> Self {
Self::with_workspace(on_event, WorkspaceContext::default(), None)
}

/// A fold seeded with repository position retained by a resumed thread.
pub(super) fn with_workspace(
on_event: Option<OnEvent>,
workspace_context: WorkspaceContext,
on_workspace_context: Option<OnWorkspaceContext>,
) -> Self {
Self {
reply: String::new(),
items: 0,
usage: None,
error: None,
last_activity: Instant::now(),
on_event,
workspace_context,
on_workspace_context,
line: 0,
}
}
Expand Down Expand Up @@ -108,6 +126,7 @@ impl FoldState {
self.emit("item/completed", "agent_message", json!({ "text": text }));
}
}
self.capture_worktree(item);
false
}
"thread/tokenUsage/updated" => {
Expand Down Expand Up @@ -188,6 +207,41 @@ impl FoldState {
on_event(&event);
}
}

/// Persist a stable worktree report carried by a completed command item.
fn capture_worktree(&mut self, item: Option<&Value>) {
let Some(item) = item
.filter(|item| item.get("type").and_then(Value::as_str) == Some("commandExecution"))
else {
return;
};
let output = item
.get("aggregatedOutput")
.or_else(|| item.get("aggregated_output"))
.and_then(Value::as_str)
.unwrap_or("");
let Some((cwd, branch)) = worktree_checkout_from_output(output) else {
return;
};
if self.workspace_context.cwd.as_deref() != Some(&cwd)
|| self.workspace_context.branch.as_deref() != Some(&branch)
{
self.workspace_context.pull_request = None;
}
self.workspace_context.cwd = Some(cwd);
self.workspace_context.branch = Some(branch);
self.emit(
"item/completed:workspace",
crate::harness_work::kinds::SESSION_INFO,
json!({
"cwd": self.workspace_context.cwd,
"branch": self.workspace_context.branch,
}),
);
if let Some(callback) = self.on_workspace_context.as_ref() {
callback(self.workspace_context.clone());
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}

/// A `status` payload saying the lane is working, with a one-line detail.
Expand Down
52 changes: 51 additions & 1 deletion src/sdk/src/daemon/providers/codex_server/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use serde_json::json;
use crate::codex_app_server::Notification;
use crate::daemon::mappers::HarnessSemanticEvent;
use crate::protocol::{HarnessProvider, HarnessTransport};
use crate::sessions::SessionClass;
use crate::sessions::{SessionClass, WorkspaceContext};

use super::super::types::{Abort, RunTaskOptions};
use super::execution::{child_env, uses_app_server, HARNESS_TRANSPORT_ENV};
Expand Down Expand Up @@ -288,3 +288,53 @@ fn advances_the_event_ordering_key() {
.iter()
.all(|event| event.record_type.starts_with("app_server:")));
}

#[test]
fn completed_worktree_command_updates_the_app_server_workspace() {
let seen = Arc::new(Mutex::new(Vec::new()));
let sink = seen.clone();
let events = Arc::new(Mutex::new(Vec::new()));
let event_sink = events.clone();
let mut fold = FoldState::with_workspace(
Some(Box::new(move |event| {
event_sink.lock().unwrap().push(event.clone())
})),
WorkspaceContext::default(),
Some(Box::new(move |context| sink.lock().unwrap().push(context))),
);
fold.fold(&notification(
"item/completed",
json!({
"item": {
"type": "commandExecution",
"command": "worktree fix-context --json",
"aggregatedOutput": concat!(
"{\"status\":\"ready\",\"repository\":\"/repo\",",
"\"path\":\"/repo/worktrees/fix-context\",",
"\"branch\":\"fix-context\",\"head\":\"abc123456789\",",
"\"headShort\":\"abc1234\",\"created\":true,",
"\"submodules\":{\"state\":\"initialized_recursive\",\"count\":0},",
"\"nextCommand\":\"cd /repo/worktrees/fix-context\"}"
),
"exitCode": 0
}
}),
));

let contexts = seen.lock().unwrap();
assert_eq!(contexts.len(), 1);
assert_eq!(
contexts[0].cwd.as_deref(),
Some("/repo/worktrees/fix-context")
);
assert_eq!(contexts[0].branch.as_deref(), Some("fix-context"));
let events = events.lock().unwrap();
let workspace = events
.iter()
.find(|event| event.event.kind == crate::harness_work::kinds::SESSION_INFO)
.expect("the active session is told about the move");
assert_eq!(
workspace.event.payload["cwd"],
"/repo/worktrees/fix-context"
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
33 changes: 28 additions & 5 deletions src/sdk/src/daemon/providers/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,16 @@ pub fn with_auth_hint(message: &str) -> String {
/// Run one delegated task headlessly, retrying transient opencode SQLite-lock
/// exits with jittered exponential backoff.
pub async fn run_provider_task(mut options: RunTaskOptions) -> Result<RunTaskResult, String> {
// Ahead of everything, including the credential scrub below. Every line
// after this one prepares a *child process* — its environment, its router,
// its argv — and OpenHuman has no child: the turn runs in this process
// against the embedded core. Scrubbing the core's own workspace out of the
// environment on its way to the core would be exactly backwards.
// A resumed conversation may have created and moved into a linked worktree
// during an earlier turn. The mapper persists that move separately from the
// daemon's configured launch checkout; make it the runtime cwd before any
// transport consumes the options, or Codex's next turn and all of its
// defaulted tool calls silently snap back to where the daemon started.
options.cwd = effective_cwd(&options.cwd, &options.workspace_context);
// Ahead of all child-process preparation, including the credential scrub
// below. OpenHuman has no child: the turn runs in this process against the
// embedded core. Scrubbing the core's own workspace out of the environment
// on its way to the core would be exactly backwards.
if super::openhuman::uses_embedded_core(&options) {
return super::openhuman::run_openhuman_task(options).await;
}
Expand Down Expand Up @@ -225,6 +230,24 @@ pub async fn run_provider_task(mut options: RunTaskOptions) -> Result<RunTaskRes
}
}

/// Resolve the directory a resumed run should actually execute in.
///
/// A retained workspace is authoritative only while it still names a directory.
/// Worktrees can be removed between turns, in which case the configured launch
/// checkout is the only viable fallback; passing the stale path to
/// `Command::current_dir` would prevent the harness from starting at all.
pub(super) fn effective_cwd(
configured: &str,
workspace_context: &crate::sessions::WorkspaceContext,
) -> String {
workspace_context
.cwd
.as_deref()
.filter(|cwd| std::path::Path::new(cwd).is_dir())
.unwrap_or(configured)
.to_string()
}

/// A cheap uniform-ish `[0,1)` sample (no `rand` dep): folds the wall clock.
pub(super) fn rand_unit() -> f64 {
let nanos = std::time::SystemTime::now()
Expand Down
34 changes: 34 additions & 0 deletions src/sdk/src/daemon/providers/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::sync::{Arc, Mutex};
use std::time::Duration;

use crate::protocol::HarnessProvider;
use crate::sessions::WorkspaceContext;

use super::detect::{
build_run_args, detect_providers, make_path_lookup, provider_bin, provider_name,
Expand Down Expand Up @@ -137,6 +138,39 @@ fn build_run_args_per_provider() {
);
}

#[test]
fn a_retained_worktree_becomes_the_resumed_runs_working_directory() {
let launch = tempfile::tempdir().unwrap();
let worktree = tempfile::tempdir().unwrap();
let context = WorkspaceContext {
cwd: Some(worktree.path().to_string_lossy().into_owned()),
branch: Some("feature".into()),
pull_request: None,
};

assert_eq!(
super::execute::effective_cwd(launch.path().to_str().unwrap(), &context),
worktree.path().to_string_lossy(),
);
}

#[test]
fn a_removed_retained_worktree_falls_back_to_the_configured_workspace() {
let launch = tempfile::tempdir().unwrap();
let removed = tempfile::tempdir().unwrap();
let context = WorkspaceContext {
cwd: Some(removed.path().to_string_lossy().into_owned()),
branch: Some("gone".into()),
pull_request: None,
};
drop(removed);

assert_eq!(
super::execute::effective_cwd(launch.path().to_str().unwrap(), &context),
launch.path().to_string_lossy(),
);
}

#[test]
fn build_run_args_neutralizes_dash_prompt() {
let args = build_run_args(HarnessProvider::Codex, "-rf /", None, None, &[], false);
Expand Down
20 changes: 20 additions & 0 deletions src/sdk/tests/e2e_codex_app_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,26 @@ async fn runs_a_turn_and_reports_the_reply() {
assert!(kinds.contains(&"status"), "{kinds:?}");
}

#[tokio::test]
async fn resumed_turn_uses_the_worktree_as_its_runtime_cwd() {
let dir = TempDir::new();
let worktree = dir.path().join("worktree");
std::fs::create_dir_all(&worktree).unwrap();
let fake = fake_app_server(&dir, TurnScript::Reply("ok"));
let (mut options, _) = options(&fake, &home(&dir, "cwd"), "continue", 10_000);
options.cwd = dir.path().to_string_lossy().into_owned();
options.workspace_context.cwd = Some(worktree.to_string_lossy().into_owned());

run_provider_task(options).await.expect("the turn runs");

let turn = fake
.requests()
.into_iter()
.find(|request| request["method"] == "turn/start")
.expect("turn/start request");
assert_eq!(turn["params"]["cwd"], worktree.to_string_lossy().as_ref());
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/// The whole point: several tasks, one process.
#[tokio::test]
async fn shares_one_process_across_sequential_tasks() {
Expand Down
Loading