Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ jobs:
env:
CARGO_TERM_COLOR: always
BUILD_PROFILE: debug
WHITAKER_INSTALLER_VERSION: '0.2.5'
steps:
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0
- name: Setup Rust
Expand Down Expand Up @@ -41,6 +42,24 @@ jobs:
- name: Audit dependencies
if: github.actor != 'dependabot[bot]'
run: make audit
- name: Cache Whitaker installer
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
with:
path: |
~/.cargo/bin/whitaker-installer
~/.cache/cargo-binstall
key: whitaker-installer-${{ runner.os }}-${{ runner.arch }}-${{ env.WHITAKER_INSTALLER_VERSION }}
- name: Install Whitaker
run: |
if ! command -v whitaker-installer >/dev/null 2>&1; then
if cargo binstall --version >/dev/null 2>&1; then
cargo binstall --no-confirm --locked "whitaker-installer@${WHITAKER_INSTALLER_VERSION}"
else
echo "cargo-binstall unavailable; building whitaker-installer from crates.io"
cargo install --locked whitaker-installer --version "${WHITAKER_INSTALLER_VERSION}"
fi
fi
whitaker-installer
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- name: Lint
run: make lint
- name: No-CLI compile check
Expand Down
4 changes: 3 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ CARGO_FLAGS ?= --all-targets --all-features
CLIPPY_FLAGS ?= $(CARGO_FLAGS) -- $(RUST_FLAGS)
TEST_FLAGS ?= $(CARGO_FLAGS)
MDLINT ?= $(shell command -v markdownlint-cli2 2>/dev/null || printf '%s' "$$HOME/.bun/bin/markdownlint-cli2")
WHITAKER ?= whitaker
NIXIE ?= nixie

build: target/debug/$(TARGET) ## Build debug binary
Expand All @@ -27,9 +28,10 @@ test: ## Run tests with warnings treated as errors
target/%/$(TARGET): ## Build binary in debug or release mode
$(CARGO) build $(BUILD_JOBS) $(if $(findstring release,$(@)),--release) --bin $(TARGET)

lint: ## Run Clippy with warnings denied
lint: ## Run Clippy and the Whitaker Dylint suite with warnings denied
RUSTDOCFLAGS="$(RUSTDOC_FLAGS)" $(CARGO) doc --no-deps
$(CARGO) clippy $(CLIPPY_FLAGS)
RUSTFLAGS="$(RUST_FLAGS)" $(WHITAKER) --all -- $(CARGO_FLAGS)

typecheck: ## Type-check the workspace
RUSTFLAGS="$(RUST_FLAGS)" $(CARGO) check $(CARGO_FLAGS) $(BUILD_JOBS)
Expand Down
13 changes: 10 additions & 3 deletions src/api/configure_git_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ pub fn configure_container_git_identity<C: ContainerExecClient + Sync, R: HostCo

#[cfg(test)]
mod tests {
//! Unit tests for container git-identity configuration.

use super::*;
use crate::engine::test_helpers::{failure_output, success_output};
use crate::engine::{CreateExecFuture, InspectExecFuture, ResizeExecFuture, StartExecFuture};
Expand Down Expand Up @@ -139,20 +141,23 @@ mod tests {

/// Shared test plumbing: creates a Tokio runtime, builds
/// `GitIdentityParams`, and calls `configure_container_git_identity`.
///
/// Returns an [`io::Error`] when the Tokio runtime cannot be created;
/// fixtures propagate arrangement failures instead of panicking.
fn invoke(
host_runner: &MockHostRunner,
exec_client: &MockExecClient,
container_id: &str,
) -> PodbotResult<GitIdentityResult> {
let runtime = tokio::runtime::Runtime::new().expect("test requires a Tokio runtime");
) -> io::Result<PodbotResult<GitIdentityResult>> {
let runtime = tokio::runtime::Runtime::new()?;
let handle = runtime.handle().clone();
let params = GitIdentityParams {
client: exec_client,
host_runner,
container_id,
runtime_handle: &handle,
};
configure_container_git_identity(&params)
Ok(configure_container_git_identity(&params))
}

#[test]
Expand All @@ -163,6 +168,7 @@ mod tests {
let exec_client = make_exec_client(0);

let result = invoke(&host_runner, &exec_client, "sandbox-unit")
.expect("test requires a Tokio runtime")
.expect("should succeed with Configured");

assert!(
Expand All @@ -179,6 +185,7 @@ mod tests {
let exec_client = MockExecClient::new(); // no exec calls expected

let result = invoke(&host_runner, &exec_client, "sandbox-unit-2")
.expect("test requires a Tokio runtime")
.expect("should succeed with NoneConfigured");

assert!(
Expand Down
2 changes: 2 additions & 0 deletions src/api/repository_clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ impl AskpassPath {

#[cfg(test)]
mod tests {
//! Unit and property tests for repository-clone request value types.

use super::{AskpassPath, BranchName, RepositoryRef, WorkspacePath};
use crate::error::{ConfigError, PodbotError};
use proptest::prelude::*;
Expand Down
56 changes: 35 additions & 21 deletions src/config/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,29 +216,43 @@ fn build_overrides(overrides: &ConfigOverrides) -> Result<serde_json::Value> {
json_overrides.insert("image".to_owned(), serde_json::Value::String(image.clone()));
}

if overrides.agent_kind.is_some() || overrides.agent_mode.is_some() {
let mut agent = serde_json::Map::new();

if let Some(kind) = overrides.agent_kind {
agent.insert(
"kind".to_owned(),
serde_json::to_value(kind).map_err(|error| ConfigError::ParseError {
message: format!("failed to serialize agent kind override: {error}"),
})?,
);
}

if let Some(mode) = overrides.agent_mode {
agent.insert(
"mode".to_owned(),
serde_json::to_value(mode).map_err(|error| ConfigError::ParseError {
message: format!("failed to serialize agent mode override: {error}"),
})?,
);
}

if let Some(agent) = build_agent_overrides(overrides)? {
json_overrides.insert("agent".to_owned(), serde_json::Value::Object(agent));
}

Ok(serde_json::Value::Object(json_overrides))
}

/// Build the nested agent override object, if any agent override is set.
fn build_agent_overrides(
overrides: &ConfigOverrides,
) -> Result<Option<serde_json::Map<String, serde_json::Value>>> {
if overrides.agent_kind.is_none() && overrides.agent_mode.is_none() {
return Ok(None);
}

let mut agent = serde_json::Map::new();

if let Some(kind) = overrides.agent_kind {
agent.insert("kind".to_owned(), serialize_agent_override("kind", kind)?);
}

if let Some(mode) = overrides.agent_mode {
agent.insert("mode".to_owned(), serialize_agent_override("mode", mode)?);
}

Ok(Some(agent))
}

/// Serialize a single agent override field, labelling failures by field name.
fn serialize_agent_override<T: serde::Serialize>(
field: &str,
value: T,
) -> Result<serde_json::Value> {
serde_json::to_value(value).map_err(|error| {
ConfigError::ParseError {
message: format!("failed to serialize agent {field} override: {error}"),
}
.into()
})
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
2 changes: 2 additions & 0 deletions src/engine/connection/error_classification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ fn io_error_kind_in_chain(error: &dyn std::error::Error) -> Option<std::io::Erro

#[cfg(test)]
mod tests {
//! Unit tests for container-engine error classification.

use std::fmt;

use rstest::rstest;
Expand Down
118 changes: 118 additions & 0 deletions src/engine/connection/exec/acp_frame_split_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
//! Exhaustive chunk-split reassembly tests for the streaming Agentic Control
//! Protocol (ACP) frame assembler.
//!
//! These tests build a deterministic multi-frame byte stream and verify that
//! every two-way split, and a broad selection of three-way splits, reassemble
//! byte-identically through [`OutboundFrameAssembler`].

use ortho_config::serde_json;
use rstest::rstest;

use super::{assembler, collect_forward_bytes, permitted_frame};

/// Build a deterministic byte sequence containing several permitted ACP
/// frames. The sequence is reused across the exhaustive-split parameterized
/// tests below.
fn permitted_stream() -> Result<Vec<u8>, serde_json::Error> {
let frames = [
permitted_frame("session/new", b"\n")?,
permitted_frame("session/update", b"\n")?,
permitted_frame("session/cancel", b"\n")?,
permitted_frame("session/new", b"\r\n")?,
permitted_frame("session/update", b"\n")?,
];
Ok(frames.into_iter().fold(Vec::new(), |mut acc, mut frame| {
acc.append(&mut frame);
acc
}))
}

fn assemble_with_two_chunks(stream: &[u8], split_at: usize) -> Vec<u8> {
let mut framer = assembler();
let first = stream.get(..split_at).unwrap_or_default();
let second = stream.get(split_at..).unwrap_or_default();
let mut outputs = Vec::new();
let (chunk_one, fallback_one) = framer.ingest_chunk(first);
assert!(fallback_one.is_none());
outputs.extend(chunk_one);
let (chunk_two, fallback_two) = framer.ingest_chunk(second);
assert!(fallback_two.is_none());
outputs.extend(chunk_two);
assert!(framer.finish().is_none());
collect_forward_bytes(&outputs)
}

fn assemble_with_three_chunks(stream: &[u8], first_split: usize, second_split: usize) -> Vec<u8> {
assert!(first_split <= second_split);
let mut framer = assembler();
let first = stream.get(..first_split).unwrap_or_default();
let second = stream.get(first_split..second_split).unwrap_or_default();
let third = stream.get(second_split..).unwrap_or_default();
let mut outputs = Vec::new();
for chunk in [first, second, third] {
let (chunk_outputs, fallback) = framer.ingest_chunk(chunk);
assert!(fallback.is_none());
outputs.extend(chunk_outputs);
}
assert!(framer.finish().is_none());
collect_forward_bytes(&outputs)
}

#[test]
fn every_two_way_split_reassembles_to_original_byte_stream() {
let stream = permitted_stream().expect("stream should serialize");
for split_at in 1..stream.len() {
let reassembled = assemble_with_two_chunks(&stream, split_at);
assert_eq!(
reassembled, stream,
"split at byte {split_at} should reassemble byte-identically",
);
}
}

#[rstest]
#[case(1, 4)]
#[case(8, 32)]
#[case(16, 64)]
#[case(32, 96)]
#[case(40, 80)]
#[case(50, 120)]
#[case(60, 100)]
#[case(70, 140)]
#[case(80, 160)]
#[case(90, 150)]
#[case(95, 145)]
#[case(100, 200)]
#[case(110, 220)]
#[case(120, 180)]
#[case(125, 230)]
#[case(130, 240)]
#[case(135, 235)]
#[case(140, 250)]
#[case(150, 260)]
#[case(155, 265)]
#[case(160, 270)]
#[case(170, 280)]
#[case(180, 290)]
#[case(190, 300)]
#[case(200, 310)]
#[case(210, 320)]
#[case(220, 325)]
#[case(225, 330)]
#[case(230, 335)]
#[case(235, 340)]
#[case(240, 345)]
#[case(250, 350)]
fn three_way_splits_reassemble_to_original_byte_stream(
#[case] first_split: usize,
#[case] second_split: usize,
) {
let stream = permitted_stream().expect("stream should serialize");
let first_clamped = first_split.min(stream.len());
let second_clamped = second_split.min(stream.len());
let reassembled = assemble_with_three_chunks(&stream, first_clamped, second_clamped);
assert_eq!(
reassembled, stream,
"three-way split at ({first_clamped}, {second_clamped}) should reassemble identically",
);
}
Loading
Loading