refactor(supervisor): pass agent proposal state explicitly#2421
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Label |
BlockedHead SHA: Gator is blocked because this PR is still marked as a draft, so the code review workflow should not run yet. Next action: @elezar, mark the PR ready for review when it is ready for gator to run the independent code review and continue CI monitoring. |
elezar
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: This maintainer-authored refactor is project-valid for OpenShell because it is concentrated in the sandbox/supervisor policy-proposal state path and removes test coupling on a process-global Cargo feature.
Head SHA: ef5b7a73a2ee131bd78793777739dfd9ad4c6856
Review findings:
- No blocking findings remain.
- Independent reviewer note: a targeted shared-clone propagation test for
AgentProposalswould strengthen coverage, but this is not blocking for gator.
Docs: Not needed because this is an internal code refactor with no direct user-facing UX, CLI, policy syntax, provider setup, or published API behavior change.
E2E: test:e2e is already applied because this touches sandbox/network supervisor policy enforcement paths. The current checks are waiting for the copy-pr mirror, and maintainer authority is available to unblock it.
Next state: gator:watch-pipeline
|
/ok to test ef5b7a7 |
SDAChess
left a comment
There was a problem hiding this comment.
Looks good, let's merge once the lint passes.
elezar
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: This maintainer-authored refactor is project-valid for OpenShell because it is concentrated in the sandbox/supervisor policy-proposal state path and removes test coupling on a process-global Cargo feature.
Head SHA: 1a57470709fc744b7b8e409fec1697a0cf3fff8e
I rechecked the latest head after @elezar pushed the clippy cleanup and after @SDAChess noted the prior SHA looked good pending lint. The current combined-supervisor path still looks sound, but the sidecar topology has one remaining proposal-state propagation issue.
Review findings:
- One warning is inline below for sidecar proposal-state propagation.
- Non-blocking suggestion: add a lifecycle test proving cloned
AgentProposalsstate is shared acrossPolicyLocalContext, L7 deny response context, and the policy poll/process install path.
Docs: Not needed because this is an internal code refactor with no direct user-facing UX, CLI, policy syntax, provider setup, or published API behavior change.
E2E: test:e2e is already applied for sandbox/network supervisor policy enforcement paths; current checks are still pending.
Next state: gator:in-review
| retained_proto, | ||
| MiddlewareRegistryStatus::Synchronized, | ||
| loaded_policy_origin, | ||
| false, |
There was a problem hiding this comment.
gator-agent
Warning: In sidecar topology this seeds the process supervisor proposal state to false, but the agent container intentionally lacks gateway credentials/env vars, so install_initial_agent_skill() cannot reliably re-fetch the setting. Meanwhile the network sidecar can enable next_steps from its own settings poll, leaving the agent told to read /etc/openshell/skills/policy_advisor.md when the process container may never install it. Please thread agent_proposals_enabled through sidecar_control::BootstrapData, include an update message for later toggles, and have the process-side watcher set the shared flag and install skills on false -> true.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
1a57470 to
359a5bb
Compare
elezar
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Validation: This maintainer-authored refactor is project-valid for OpenShell because it is concentrated in the sandbox/supervisor policy-proposal state path and removes test coupling on a process-global Cargo feature.
Head SHA: 359a5bb51693c6c24837132afbf651b1258c8af9
I rechecked the latest head after @elezar pushed the sidecar proposal-state update. I also accounted for @SDAChess's maintainer note on the earlier SHA that the change looked good pending lint, and this new head has now had an independent code review.
Review findings:
- No blocking findings remain.
- The prior sidecar proposal-state propagation finding appears resolved: bootstrap carries
agent_proposals_enabled, sidecar updates includeAgentProposalsUpdated, and the process-side watcher applies the shared flag and installs skills onfalse -> true. - Non-blocking suggestion: a targeted regression test proving the same
AgentProposalsstate gates bothpolicy.localand L7 deny-bodynext_stepswould strengthen coverage.
Docs: Not needed because this is an internal state-threading refactor with no direct user-facing UX, CLI, policy syntax, provider setup, or published API behavior change.
E2E: test:e2e is already applied for sandbox/network supervisor policy enforcement paths.
Next state: gator:watch-pipeline
|
/ok-to-test 359a5bb |
|
Label |
Summary
Replace the process-global agent policy proposal flag with explicit shared
AgentProposalsstate threaded through sandbox, process, and network supervisor runtime paths. This removes downstream dev-dependencies onopenshell-core/test-helpersand keeps tests from relying on Cargo feature unification.Related Issue
N/A
Changes
AgentProposalsshared state inopenshell-coreand remove thetest-helpersfeature.policy.local, proxy/L7 evaluation, and deny response rendering.L7EvalContextso tests remain compact as context fields grow.Testing
mise run pre-commitpasses (not run)Additional checks run:
nix develop --command cargo fmt --checknix develop --command cargo check -p openshell-supervisor-network -p openshell-sandbox -p openshell-supervisor-process -p openshell-core --testsnix develop --command cargo test -p openshell-supervisor-network policy_local::tests -- --nocapturenix develop --command cargo test -p openshell-supervisor-network deny_response_body -- --nocapturenix develop --command cargo test -p openshell-supervisor-network send_deny_response_writes_structured_json_403 -- --nocapturenix develop --command cargo test -p openshell-supervisor-network middleware_ -- --nocapturenix develop --command cargo test -p openshell-sandbox agent_proposals_setting -- --nocaptureChecklist