feat(sandbox): adopt provider-backed policy composition - #2671
Conversation
E2E tests are runningAuthorization passed for this commit. See the E2E Tests workflow for results. |
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
Site previewPreview: https://75b80ec0-site.fullsend-ai.workers.dev Commit: |
PR Summary by QodoAdopt provider-backed network policy composition for scaffold sandboxes Description
Diagram
High-Level Assessment
Files changed (31)
|
|
🤖 Review · |
76b8033 to
96bc82c
Compare
Code Review by Qodo
Context used✅ Tickets:
🎫 Adopt provider-backed policy composition to reduce harness policy duplication 🎫 Review agent should withhold approval when dependency bot PR violates project's own dependency config✅ Compliance rules (platform):
58 rules✅ Skills:
writing-user-docs, writing-adrs 1.
|
96bc82c to
2d17fc7
Compare
|
🤖 Finished Review · ❌ Failure · Started 4:26 PM UTC · Completed 4:40 PM UTC |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
ReviewWell-structured migration from duplicated per-agent network policies to composable provider profiles. The implementation is correct: parallel provider creation with proper synchronization, hash-based idempotent profile import, input validation on provider names and types, and comprehensive test coverage. ADR 0065 accurately documents the decision and trade-offs, and all current user-facing documentation has been updated. FindingsLow
Previous runReviewFindingsLow
Previous runReviewFindingsLow
Previous run (2)ReviewFindingsMedium
Previous run (3)ReviewFindingsMedium
Previous stale-policy-reference findings (docs/ADRs/0038, docs/plans/universal-harness-access.md, docs/plans/universal-harness-access-phase2.md) remain present but are downgraded — these are historical/plan documents not modified by this PR, and their references to Previous run (4)ReviewFindingsMedium
Previous findings from the prior review (command-injection in sandbox.go, cross-ADR coherence gap) have been addressed or downgraded — credential/config key validation is mitigated by Previous run (5)ReviewFindingsMedium
Previous findings from the prior review (command-injection in sandbox.go, cross-ADR coherence gap) have been addressed or downgraded — credential/config key validation is mitigated by Previous run (6)ReviewFindingsMedium
Previous findings from the prior review (concurrent error handling in run.go, cross-ADR coherence gap) have both been addressed — errors are now collected with Previous runReviewFindingsMedium
Previous findings from the prior review (stale references in architecture.md, customizing-agents.md, and ADR profile count inconsistency) have all been addressed in this revision. Labels: PR modifies Go sandbox/harness code alongside policy composition changes and documentation. Previous run (7)ReviewFindingsMedium
Labels: PR modifies sandbox policy composition, harness provider wiring, and user-facing documentation. Labels: PR modifies sandbox policy composition, harness provider declarations, and user-facing documentation (ADR, architecture, user guide). Previous run (8)ReviewFindingsMedium
Labels: PR modifies sandbox policy composition and harness provider wiring. Previous run (9)ReviewFindingsMedium
Previous findings from the prior review (concurrent error handling in run.go, cross-ADR coherence gap) have both been addressed — errors are now collected with Previous run (10)ReviewFindingsMedium
Previous findings from the prior review (stale references in architecture.md, customizing-agents.md, and ADR profile count inconsistency) have all been addressed in this revision. Labels: PR modifies Go sandbox/harness code alongside policy composition changes and documentation. Previous run (11)ReviewFindingsMedium
Labels: PR modifies sandbox policy composition, harness provider wiring, and user-facing documentation. Labels: PR modifies sandbox policy composition, harness provider declarations, and user-facing documentation (ADR, architecture, user guide). Previous run (12)ReviewFindingsMedium
Labels: PR modifies sandbox policy composition and harness provider wiring. |
2d17fc7 to
03a77ac
Compare
|
🤖 Review · |
03a77ac to
7812426
Compare
|
Addressed all three Qodo findings:
|
7812426 to
6de4719
Compare
|
🤖 Finished Review · ✅ Success · Started 5:14 PM UTC · Completed 5:30 PM UTC |
|
🤖 Finished Retro · ✅ Success · Started 9:35 AM UTC · Completed 9:43 AM UTC |
|
PR #2671 migrated duplicated per-agent network policies to composable provider profiles — a large, well-structured change (858 additions, 562 deletions, 38 files) by a human author using Claude Code. Two human reviewers approved after substantive discussion. The fullsend review agent ran 17 times (7 cancelled, 1 failed, 8 successful). Qodo caught the critical RW/RO profile split first. Human reviewers caught things agents missed: an Proposals filed
|
- Remove Ship of Theseus reference (too obscure without explanation) - Remove explicit agent list from intro (will go stale) - Broaden "the rule" to acknowledge general-purpose harness fields alongside documented extension points, resolving the inconsistency between the rule and the classification table - Drop conditional language around policy composition (PR #2671 merged) - Sweep docs/ link text: "Customizing" → "Configuring" for guides about documented extension points, reserving "custom" for from-scratch agents - Update glossary entries to match Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
- Remove Ship of Theseus reference (too obscure without explanation) - Remove explicit agent list from intro (will go stale) - Broaden "the rule" to acknowledge general-purpose harness fields alongside documented extension points, resolving the inconsistency between the rule and the classification table - Drop conditional language around policy composition (PR #2671 merged) - Sweep docs/ link text: "Customizing" → "Configuring" for guides about documented extension points, reserving "custom" for from-scratch agents - Update glossary entries to match Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Summary
Replaces duplicated network rules across harness policy files with composable provider profiles, using OpenShell's provider-backed policy composition (v0.0.37+).
providers:field; policy files shrink to non-composable sandbox restrictions only (filesystem, landlock, process)base.yamlreplaces 6 per-agent policy files — network rules come from provider profiles at sandbox fetch timefullsend runimports profiles, creates providers in parallel, enablesproviders_v2_enabled, and warns on undeclared providersOpenShell CLI workarounds
openshell settings setnow requires--key/--valueflags +--yes_NOOP_<NAME>=dummy credentials (NVIDIA/OpenShell#1978)Follow-up
Closes #776
Test plan
make go-test)base.yamlfullsend run triageagainst live repo with new provider composition🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com