Skip to content

Commit 6f79bbb

Browse files
zredlinedjohntmyers
authored andcommitted
test(policy-advisor): require proposal opt-in for e2e
1 parent 872dcc0 commit 6f79bbb

7 files changed

Lines changed: 97 additions & 62 deletions

File tree

crates/openshell-cli/src/run.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5493,10 +5493,10 @@ fn format_timestamp_ms(ms: i64) -> String {
54935493
mod tests {
54945494
use super::{
54955495
TlsOptions, dockerfile_sources_supported_for_gateway, format_endpoint,
5496-
format_gateway_select_header,
5497-
format_gateway_select_items, format_provider_attachment_table, gateway_add,
5498-
gateway_auth_label, gateway_env_override_warning, gateway_select_with, gateway_type_label,
5499-
git_sync_files, http_health_check, image_requests_gpu, import_local_package_mtls_bundle,
5496+
format_gateway_select_header, format_gateway_select_items,
5497+
format_provider_attachment_table, gateway_add, gateway_auth_label,
5498+
gateway_env_override_warning, gateway_select_with, gateway_type_label, git_sync_files,
5499+
http_health_check, image_requests_gpu, import_local_package_mtls_bundle,
55005500
inferred_provider_type, package_managed_tls_dirs, parse_cli_setting_value,
55015501
parse_credential_pairs, plaintext_gateway_is_remote, provisioning_timeout_message,
55025502
ready_false_condition_message, resolve_from, sandbox_should_persist,

crates/openshell-sandbox/src/l7/rest.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,7 +1053,7 @@ mod tests {
10531053
#[test]
10541054
fn deny_response_body_is_agent_readable_and_redacted() {
10551055
// Agent-readable next_steps is gated on the proposals feature flag.
1056-
let _proposals = crate::test_helpers::ProposalsFlagGuard::set(true);
1056+
let _proposals = crate::test_helpers::ProposalsFlagGuard::set_blocking(true);
10571057
let req = L7Request {
10581058
action: "PUT".to_string(),
10591059
target: "/repos/NVIDIA/OpenShell/contents/README.md?access_token=secret-token"
@@ -1113,7 +1113,7 @@ mod tests {
11131113
#[tokio::test]
11141114
async fn send_deny_response_writes_structured_json_403() {
11151115
// Agent-readable next_steps is gated on the proposals feature flag.
1116-
let _proposals = crate::test_helpers::ProposalsFlagGuard::set(true);
1116+
let _proposals = crate::test_helpers::ProposalsFlagGuard::set(true).await;
11171117
let (mut client, mut server) = tokio::io::duplex(4096);
11181118
let send = tokio::spawn(async move {
11191119
let req = L7Request {

crates/openshell-sandbox/src/lib.rs

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,8 @@ pub(crate) fn ocsf_ctx() -> &'static SandboxContext {
9797
/// to gate the agent-controlled mutation surface. Exposed `pub(crate)` so
9898
/// unit tests in sibling modules can flip the flag through a serialized
9999
/// guard (see `policy_local::tests::ProposalsFlagGuard`).
100-
pub(crate) static AGENT_PROPOSALS_ENABLED:
101-
OnceLock<Arc<std::sync::atomic::AtomicBool>> = OnceLock::new();
100+
pub(crate) static AGENT_PROPOSALS_ENABLED: OnceLock<Arc<std::sync::atomic::AtomicBool>> =
101+
OnceLock::new();
102102

103103
/// Read the current value of the agent proposals feature flag.
104104
///
@@ -113,17 +113,22 @@ pub(crate) fn agent_proposals_enabled() -> bool {
113113
/// Test-only helpers shared across sibling test modules.
114114
#[cfg(test)]
115115
pub(crate) mod test_helpers {
116-
#![allow(clippy::redundant_pub_crate, reason = "intentional crate-private module")]
116+
#![allow(
117+
clippy::redundant_pub_crate,
118+
reason = "intentional crate-private module"
119+
)]
117120
use std::sync::Arc;
118-
use std::sync::Mutex;
119-
use std::sync::MutexGuard;
121+
use std::sync::LazyLock;
120122
use std::sync::atomic::{AtomicBool, Ordering};
123+
use tokio::sync::MutexGuard;
124+
125+
static PROPOSALS_FLAG_LOCK: LazyLock<tokio::sync::Mutex<()>> =
126+
LazyLock::new(|| tokio::sync::Mutex::new(()));
121127

122128
/// Guard for tests that toggle the process-wide
123-
/// `AGENT_PROPOSALS_ENABLED` flag. Acquires a process-wide mutex on
124-
/// construction so concurrent tests don't race on the atomic, swaps in
125-
/// the requested value, and restores the previous value on drop. Hold
126-
/// the guard for the duration of any code that reads
129+
/// `AGENT_PROPOSALS_ENABLED` flag. Acquires a process-wide async mutex,
130+
/// swaps in the requested value, and restores the previous value on drop.
131+
/// Hold the guard for the duration of any code that reads
127132
/// `agent_proposals_enabled()`.
128133
pub(crate) struct ProposalsFlagGuard {
129134
prev: bool,
@@ -132,9 +137,17 @@ pub(crate) mod test_helpers {
132137
}
133138

134139
impl ProposalsFlagGuard {
135-
pub(crate) fn set(enabled: bool) -> Self {
136-
static LOCK: Mutex<()> = Mutex::new(());
137-
let lock = LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
140+
pub(crate) async fn set(enabled: bool) -> Self {
141+
let lock = PROPOSALS_FLAG_LOCK.lock().await;
142+
Self::with_lock(enabled, lock)
143+
}
144+
145+
pub(crate) fn set_blocking(enabled: bool) -> Self {
146+
let lock = PROPOSALS_FLAG_LOCK.blocking_lock();
147+
Self::with_lock(enabled, lock)
148+
}
149+
150+
fn with_lock(enabled: bool, lock: MutexGuard<'static, ()>) -> Self {
138151
let flag = super::AGENT_PROPOSALS_ENABLED
139152
.get_or_init(|| Arc::new(AtomicBool::new(false)))
140153
.clone();
@@ -394,7 +407,10 @@ pub async fn run_sandbox(
394407
// gates the skill install, the policy.local route handler, and the L7
395408
// deny body's `next_steps` field — see `agent_proposals_enabled()`.
396409
let proposals_enabled = Arc::new(std::sync::atomic::AtomicBool::new(false));
397-
if AGENT_PROPOSALS_ENABLED.set(proposals_enabled.clone()).is_err() {
410+
if AGENT_PROPOSALS_ENABLED
411+
.set(proposals_enabled.clone())
412+
.is_err()
413+
{
398414
debug!("agent proposals flag already initialized, keeping existing");
399415
}
400416

@@ -426,9 +442,7 @@ pub async fn run_sandbox(
426442
}
427443
}
428444
} else {
429-
debug!(
430-
"agent_policy_proposals_enabled is false at startup; skipping skill install"
431-
);
445+
debug!("agent_policy_proposals_enabled is false at startup; skipping skill install");
432446
}
433447

434448
// Generate ephemeral CA and TLS state for HTTPS L7 inspection.

crates/openshell-sandbox/src/policy_local.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -360,10 +360,7 @@ fn is_ocsf_denial_line(line: &str) -> bool {
360360
line.contains(" OCSF ") && line.contains(" DENIED ")
361361
}
362362

363-
fn collect_shorthand_log_files(
364-
log_dir: &Path,
365-
max_files: usize,
366-
) -> std::io::Result<Vec<PathBuf>> {
363+
fn collect_shorthand_log_files(log_dir: &Path, max_files: usize) -> std::io::Result<Vec<PathBuf>> {
367364
let mut entries: Vec<(std::time::SystemTime, PathBuf)> = std::fs::read_dir(log_dir)?
368365
.filter_map(std::result::Result::ok)
369366
.filter_map(|entry| {
@@ -1075,7 +1072,7 @@ mod tests {
10751072

10761073
#[test]
10771074
fn agent_next_steps_returns_empty_when_flag_off() {
1078-
let _guard = ProposalsFlagGuard::set(false);
1075+
let _guard = ProposalsFlagGuard::set_blocking(false);
10791076
let steps = agent_next_steps();
10801077
let arr = steps.as_array().expect("agent_next_steps is an array");
10811078
assert!(
@@ -1086,7 +1083,7 @@ mod tests {
10861083

10871084
#[test]
10881085
fn agent_next_steps_returns_full_array_when_flag_on() {
1089-
let _guard = ProposalsFlagGuard::set(true);
1086+
let _guard = ProposalsFlagGuard::set_blocking(true);
10901087
let steps = agent_next_steps();
10911088
let arr = steps.as_array().expect("agent_next_steps is an array");
10921089
assert_eq!(arr.len(), 4, "expected 4 next_steps when feature is on");
@@ -1100,7 +1097,7 @@ mod tests {
11001097

11011098
#[tokio::test]
11021099
async fn route_request_returns_feature_disabled_when_flag_off() {
1103-
let _guard = ProposalsFlagGuard::set(false);
1100+
let _guard = ProposalsFlagGuard::set(false).await;
11041101
let ctx = PolicyLocalContext::new(
11051102
Some(ProtoSandboxPolicy {
11061103
version: 1,
@@ -1127,7 +1124,7 @@ mod tests {
11271124

11281125
#[tokio::test]
11291126
async fn current_policy_route_returns_yaml_envelope() {
1130-
let _guard = ProposalsFlagGuard::set(true);
1127+
let _guard = ProposalsFlagGuard::set(true).await;
11311128
let ctx = PolicyLocalContext::new(
11321129
Some(ProtoSandboxPolicy {
11331130
version: 1,

e2e/policy-advisor/README.md

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,37 @@ runs the same loop with Codex driving from inside the sandbox.
1818

1919
## Run it
2020

21+
Run against an ephemeral Docker gateway:
22+
23+
```bash
24+
DEMO_GITHUB_OWNER=<your-handle> \
25+
DEMO_GITHUB_REPO=openshell-policy-demo \
26+
e2e/with-docker-gateway.sh bash -lc '
27+
target/debug/openshell settings set --global \
28+
--key agent_policy_proposals_enabled \
29+
--value true \
30+
--yes
31+
OPENSHELL_BIN="$PWD/target/debug/openshell" bash e2e/policy-advisor/test.sh
32+
'
33+
```
34+
35+
To keep the sandbox for debugging, start a local gateway first with
36+
`mise run gateway:docker`, then run:
37+
2138
```bash
39+
target/debug/openshell settings set --global \
40+
--key agent_policy_proposals_enabled \
41+
--value true \
42+
--yes
43+
44+
OPENSHELL_GATEWAY=docker-dev \
45+
OPENSHELL_BIN="$PWD/target/debug/openshell" \
46+
DEMO_KEEP_SANDBOX=1 \
2247
DEMO_GITHUB_OWNER=<your-handle> \
2348
DEMO_GITHUB_REPO=openshell-policy-demo \
2449
bash e2e/policy-advisor/test.sh
2550
```
2651

27-
Requires an active OpenShell gateway (`openshell gateway start`) and a GitHub
28-
token with contents write on the repository (auto-resolved from `gh auth token`,
29-
`GITHUB_TOKEN`, or `GH_TOKEN`).
52+
Requires Docker, `agent_policy_proposals_enabled=true`, and a GitHub token with
53+
contents write on the repository. The test auto-resolves the token from
54+
`DEMO_GITHUB_TOKEN`, `GITHUB_TOKEN`, `GH_TOKEN`, or `gh auth token`.

e2e/policy-advisor/test.sh

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ DEMO_KEEP_SANDBOX="${DEMO_KEEP_SANDBOX:-0}"
2828
DEMO_RETRY_ATTEMPTS="${DEMO_RETRY_ATTEMPTS:-30}"
2929
DEMO_RETRY_SLEEP="${DEMO_RETRY_SLEEP:-2}"
3030

31-
TMP_DIR="$(mktemp -d "${TMPDIR:-/tmp}/openshell-agent-policy.XXXXXX")"
32-
POLICY_FILE="${TMP_DIR}/policy.yaml"
33-
SSH_CONFIG="${TMP_DIR}/ssh_config"
31+
TMP_DIR=""
32+
POLICY_FILE=""
33+
SSH_CONFIG=""
3434
SSH_HOST=""
3535

3636
BOLD='\033[1m'
@@ -63,20 +63,12 @@ cleanup() {
6363
printf "\n${YELLOW}Keeping sandbox because DEMO_KEEP_SANDBOX=1: %s${RESET}\n" "$DEMO_SANDBOX_NAME"
6464
fi
6565

66-
# Restore the agent_policy_proposals_enabled setting to what it was
67-
# before this run. We saved the prior value in $PRIOR_PROPOSALS_FLAG.
68-
if [[ -n "${PRIOR_PROPOSALS_FLAG:-}" ]]; then
69-
if [[ "$PRIOR_PROPOSALS_FLAG" == "(unset)" ]]; then
70-
"$OPENSHELL_BIN" settings delete --global --key agent_policy_proposals_enabled \
71-
>/dev/null 2>&1 || true
72-
else
73-
"$OPENSHELL_BIN" settings set --global --key agent_policy_proposals_enabled \
74-
--value "$PRIOR_PROPOSALS_FLAG" >/dev/null 2>&1 || true
75-
fi
76-
fi
77-
7866
"$OPENSHELL_BIN" provider delete "$DEMO_GITHUB_PROVIDER_NAME" >/dev/null 2>&1 || true
7967

68+
if [[ -z "$TMP_DIR" ]]; then
69+
return
70+
fi
71+
8072
if [[ $status -eq 0 ]]; then
8173
rm -rf "$TMP_DIR"
8274
else
@@ -209,19 +201,23 @@ create_provider() {
209201
--credential GITHUB_TOKEN
210202
}
211203

212-
enable_agent_proposals() {
213-
step "Enabling agent-driven policy proposals"
214-
# Snapshot the prior value so cleanup() can restore it. Use a sentinel
215-
# for "unset" so we can distinguish from an explicit false on restore.
216-
local prior
217-
prior="$("$OPENSHELL_BIN" settings get --global --json 2>/dev/null \
218-
| grep -o '"agent_policy_proposals_enabled"[^,}]*' \
219-
| grep -o 'true\|false' | head -1)"
220-
PRIOR_PROPOSALS_FLAG="${prior:-(unset)}"
221-
info " Prior global value: $PRIOR_PROPOSALS_FLAG"
222-
"$OPENSHELL_BIN" settings set --global \
223-
--key agent_policy_proposals_enabled --value true >/dev/null \
224-
|| fail "could not set agent_policy_proposals_enabled globally"
204+
check_agent_proposals_enabled() {
205+
step "Checking agent-driven policy proposal opt-in"
206+
local value
207+
value="$("$OPENSHELL_BIN" settings get --global --json 2>/dev/null \
208+
| jq -r '.settings.agent_policy_proposals_enabled // "<unset>"')"
209+
if [[ "$value" != "true" ]]; then
210+
fail "agent_policy_proposals_enabled must be true before running this test.
211+
Enable it with:
212+
$OPENSHELL_BIN settings set --global --key agent_policy_proposals_enabled --value true --yes"
213+
fi
214+
info "${GREEN}agent_policy_proposals_enabled=true${RESET}"
215+
}
216+
217+
create_temp_workspace() {
218+
TMP_DIR="$(mktemp -d "${TMPDIR:-/tmp}/openshell-agent-policy.XXXXXX")"
219+
POLICY_FILE="${TMP_DIR}/policy.yaml"
220+
SSH_CONFIG="${TMP_DIR}/ssh_config"
225221
}
226222

227223
create_sandbox() {
@@ -393,9 +389,10 @@ show_logs() {
393389
main() {
394390
validate_env
395391
check_gateway
392+
check_agent_proposals_enabled
393+
create_temp_workspace
396394
check_github_access
397395
create_provider
398-
enable_agent_proposals
399396
create_sandbox
400397
connect_ssh
401398
run_policy_local_checks

examples/agent-driven-policy-management/agent-task.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
<!-- SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -->
22
<!-- SPDX-License-Identifier: Apache-2.0 -->
33

4+
# Agent Task
5+
46
You are running inside an OpenShell sandbox. Your job is to write one
57
markdown file to GitHub via the GitHub Contents API.
68

0 commit comments

Comments
 (0)