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
43 changes: 26 additions & 17 deletions crates/openhuman-core/src/agent/orchestration/delegation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,23 +84,7 @@ pub(crate) async fn run_subagent_delegation_with_parent_context(
//
// With a storage backend configured (`crate::storage`) the checkpoints
// live there instead, in the acting agent's scope.
let checkpointer: Arc<dyn Checkpointer<DelegationState>> =
match crate::storage::current_scoped()
.map_err(|e| format!("resolve the storage scope for graph checkpoints: {e}"))?
{
Some(scoped) => Arc::new(tinyagents_graph::checkpoint::DriverCheckpointer::<
DelegationState,
>::with_prefix(
Arc::clone(scoped.documents()), "delegation_graph"
)),
None => {
let checkpoint_db = config.workspace_dir.join("graph_checkpoints.db");
Arc::new(
SqliteCheckpointer::<DelegationState>::open(&checkpoint_db)
.map_err(|e| format!("open durable graph checkpoint store: {e}"))?,
)
}
};
let checkpointer = open_delegation_checkpointer(&config)?;

tracing::info!(
target: LOG_TARGET,
Expand Down Expand Up @@ -311,6 +295,31 @@ fn delegation_subagent_options(
}
}

/// Open the delegation graph's durable checkpointer: the acting agent's scope
/// on the storage backend when one is installed, else `graph_checkpoints.db`
/// under the workspace.
pub fn open_delegation_checkpointer(
config: &Config,
) -> Result<Arc<dyn Checkpointer<DelegationState>>, String> {
match crate::storage::current_scoped()
.map_err(|e| format!("resolve the storage scope for graph checkpoints: {e}"))?
{
Some(scoped) => Ok(Arc::new(
tinyagents_graph::checkpoint::DriverCheckpointer::<DelegationState>::with_prefix(
Arc::clone(scoped.documents()),
"delegation_graph",
),
)),
None => {
let checkpoint_db = config.workspace_dir.join("graph_checkpoints.db");
Ok(Arc::new(
SqliteCheckpointer::<DelegationState>::open(&checkpoint_db)
.map_err(|e| format!("open durable graph checkpoint store: {e}"))?,
))
}
}
}

#[cfg(test)]
#[path = "delegation_tests.rs"]
mod tests;
1 change: 1 addition & 0 deletions crates/openhuman-core/src/agent/orchestration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub mod command_center;
pub(crate) mod completion_notice;
pub(crate) mod completion_target;
pub(crate) mod delegation;
pub use delegation::open_delegation_checkpointer;
pub(crate) mod fleet_tools;
mod ops;
pub(crate) mod parent_context;
Expand Down
12 changes: 6 additions & 6 deletions crates/openhuman-core/src/flows/tinyflows/state_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ fn without_a_backend_the_flow_state_is_sqlite() {
workspace_dir: dir.path().to_path_buf(),
..Config::default()
};
if crate::storage::installed().is_none() {
assert!(matches!(
FlowState::open(&config, "flow:a"),
FlowState::Sqlite(_)
));
}
// Nothing in this binary installs a backend, so the slot is empty.
assert!(crate::storage::installed().is_none());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium security confident

Avoid asserting process-global state in a parallel test

crate::storage::installed() is process-wide state, so this test can fail depending on whether another test has installed a backend. The previous conditional assertion correctly skipped the SQLite check when a backend was present; preserve that isolation or explicitly control and restore the global storage state for this test.


Additional critique observation

priority medium confident

Preserve the guard around the global storage assertion

[RULE] global-test-state

crate::storage::installed() reads process-wide storage state, not state owned by this test. Another test running in the same test binary can install a backend concurrently or before this test, making this assertion fail even though FlowState::open should correctly select Documents. The previous conditional avoided coupling this test to global test order; restore that guard or otherwise isolate and serialize the global storage state.

Suggested change for the opening observation

Suggested change
assert!(crate::storage::installed().is_none());
if crate::storage::installed().is_none() {

[RULE] test-isolation ·

assert!(matches!(
FlowState::open(&config, "flow:a"),
FlowState::Sqlite(_)
));
}
7 changes: 5 additions & 2 deletions scripts/ci/check-dep-sim-calibration.sh
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,11 @@ cd "$(dirname "$0")/../.."
# 314 -> 315 on 2026-10-09: secrets on the storage backend add the
# first-party tinystoragedrivers-secrets crate (its crypto deps were already
# in the graph).
# This matches the current `flows:337:315:2` entry in
# 315 -> 316 on 2026-10-09: cron and flows on the storage ports add the
# first-party tinyflows-drivers crate (one package, one name; no new
# external crate).
# This matches the current `flows:338:316:2` entry in
# scripts/kernel-floor.limits; its preceding entries are historical.
EXPECTED_NAMES=315
EXPECTED_NAMES=316

exec python3 scripts/dep-sim.py --cut-nothing --expect-names "${EXPECTED_NAMES}"
5 changes: 4 additions & 1 deletion scripts/kernel-floor.limits
Original file line number Diff line number Diff line change
Expand Up @@ -665,4 +665,7 @@
# from the Linux flows graph; native builds remain at 3.
# 337/315/2 2026-10-09 secrets on the storage backend add the first-party
# tinystoragedrivers-secrets crate (one package, one name).
flows:337:315:2
# 338/316/2 2026-10-09 cron and flows on the storage ports add the first-party
# tinyflows-drivers crate (one package, one name; no new
# external crate, native builds unchanged).
flows:338:316:2
18 changes: 17 additions & 1 deletion tests/storage_flows_e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ async fn a_configured_backend_holds_cron_flows_and_flow_state() {
.await
.unwrap();

for path in ["cron/jobs.db", "flows/flows.db"] {
// The delegation graph's checkpointer lives on the backend too.
let delegation_config = config.clone();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium tests uncertain

Backend path of the delegation checkpointer is opened but never verified

This call constructs a DriverCheckpointer and immediately drops it; the only assertion afterwards is that graph_checkpoints.db does not exist in the workspace, which would also pass if the function silently did nothing with the backend. The invariant the comment claims — "The delegation graph's checkpointer lives on the backend too" — needs a test that would fail if the scoped-backend branch regressed to something else: write a checkpoint through the returned checkpointer (or at least confirm the driver-backed prefix is reachable via crate::storage::current_scoped()), and read it back. As written, the backend branch of open_delegation_checkpointer has no failing assertion.

[RULE] test-asserts-nothing ·

openhuman_core::agent::orchestration::open_delegation_checkpointer(&delegation_config).unwrap();

for path in ["cron/jobs.db", "flows/flows.db", "graph_checkpoints.db"] {
assert!(
!workspace.path().join(path).exists(),
"{path} was not written"
Expand All @@ -84,9 +88,21 @@ async fn a_configured_backend_holds_cron_flows_and_flow_state() {

// Without a backend the classic databases are back in use.
assert!(openhuman_core::storage::clear());
let fallback = config.clone();
tokio::task::spawn_blocking(move || {
assert!(matches!(
FlowState::open(&fallback, "flow:digest"),
FlowState::Sqlite(_)
));
})
.await
.unwrap();
// The flow written to the backend is not in the classic catalog.
assert!(flows::ops::flows_list(&config)
.await
.unwrap()
.value
.is_empty());
openhuman_core::agent::orchestration::open_delegation_checkpointer(&config).unwrap();
assert!(workspace.path().join("graph_checkpoints.db").exists());
}
Loading