Skip to content

Add agent execution sandbox architecture ADR - #659

Merged
ggallen merged 15 commits into
fullsend-ai:mainfrom
ggallen:adr-0029-agent-sandbox
May 11, 2026
Merged

Add agent execution sandbox architecture ADR#659
ggallen merged 15 commits into
fullsend-ai:mainfrom
ggallen:adr-0029-agent-sandbox

Conversation

@ggallen

@ggallen ggallen commented May 5, 2026

Copy link
Copy Markdown
Member

This ADR addresses the agent execution environment question deferred in the GitLab support design (ADR-0028).

Summary

Proposes a shared container image (ghcr.io/fullsend-ai/agent-sandbox) for agent execution that works on both GitHub Actions and GitLab CI. The image contains OpenShell, the fullsend agent harness, and a curated set of tools.

Key Decisions

  • Shared container image across GitHub Actions and GitLab CI (ensures environment parity, which is a security requirement)
  • Supported executors: GitHub Actions VM runners, GitLab Docker executor, GitLab Kubernetes executor
  • OpenShell as PID 1 with nested sandboxes for individual agent processes
  • Privileged container required for OpenShell network namespace manipulation (documented alternatives: rootless Podman, user namespaces)

Contents

  • ADR-0036 (docs/ADRs/0036-agent-execution-sandbox.md) - Architectural decision covering context, options (shared image vs platform-specific vs Kubernetes CRDs vs minimal sandbox), decision rationale, and consequences
  • Implementation document (docs/plans/agent-execution-environment.md) - Detailed implementation guidance including container image build pipeline, OpenShell configuration, resource limits, GitLab runner setup, image signing, and upgrade procedures

Relationship to Existing ADRs

  • Builds on ADR-0028 (GitLab support) by addressing the explicitly deferred agent execution environment question
  • References ADR-0017 (credential isolation) and ADR-0025 (provider credential delivery) for credential delivery mechanisms
  • Relates to agent-infrastructure.md but is more specific (container image design vs platform strategy)

Open Questions

  • Rootless OpenShell support (user namespace remapping vs eBPF-based L7 enforcement)
  • Image build and distribution strategy (public registry vs per-org builds)
  • Builder services for Docker-in-Docker use cases (external Kaniko vs prohibit container builds)

Status: Proposed (design exploration, seeking feedback)

@fullsend-ai-review

fullsend-ai-review Bot commented May 5, 2026

Copy link
Copy Markdown

Review: #659

Head SHA: 8a5c3e4
Timestamp: 2026-05-11T00:00:00Z
Outcome: approve

Summary

This PR adds ADR-0036 (Agent Execution Sandbox Architecture) and a companion implementation plan for the agent execution environment. The ADR addresses the question explicitly deferred in ADR-0028 (GitLab Support) — verified against ADR-0028 §5 which scopes out agent execution environment as orthogonal to dispatch. The decision to use a shared container image (Docker-first) is well-reasoned, with clear trade-off analysis against three alternatives (platform-specific images, Kubernetes CRDs, minimal sandbox + dynamic tools). The implementation plan correctly defers to ADR-0030 as the authoritative OpenShell interaction model with a prominent disclaimer. The README update adds a previously missing docs/plans/ section linking to both new and pre-existing plan documents. No critical, high, or medium findings. The PR is well-scoped documentation that follows repo conventions.

Findings

Info

  • [style/conventions] docs/ADRs/0036-agent-execution-sandbox.md — ADR numbered 0036 while the highest existing ADR on main is 0034 (no 0035 exists). This is consistent with existing gaps in the repo (no 0029, no 0033), so no action needed, but worth noting for future ADR authors to avoid confusion.

  • [correctness] docs/plans/agent-execution-environment.md:371 — The illustrative Dockerfile downloads OpenShell via curl without checksum verification (curl -L ... -o /usr/local/bin/openshell). The doc is a design exploration (not production code), and there's a disclaimer that examples are illustrative, but when this moves to implementation, the Dockerfile should verify checksums or use a signed artifact.

  • [correctness] docs/plans/agent-execution-environment.md:928 — The bearer token example uses uuidgen for the per-run token. UUIDs (v4) provide 122 bits of randomness which is adequate for ephemeral single-use tokens in this context, but implementation should consider openssl rand -hex 32 for a more conventional security token pattern.

  • [correctness] docs/plans/agent-execution-environment.md:987-990 — The GitLab CI verification example installs cosign via apt-get in before_script. Cosign is not typically available in default apt repositories — implementation would need the Sigstore apt repo or a binary download. This is an illustrative example so no action required now.

  • [intent alignment] README.md — The README update adds links to two pre-existing plan documents (2026-03-06-adr46-drift-scanner.md, vertex-inference-provisioning.md) in addition to the new agent-execution-environment.md. This is useful housekeeping beyond the ADR scope and is a welcome addition.

Footer

Outcome: approve
This review applies to SHA 8a5c3e43fc3e8936a9341505a19259e9f8268c45. Any push to the PR head clears this review and requires a new evaluation.

Previous run

Review: #659

Head SHA: cb8a17f
Timestamp: 2026-05-11T00:00:00Z
Outcome: request-changes

Summary

This PR adds a well-structured ADR-0036 for the agent execution sandbox architecture and a detailed companion implementation plan. The documents are thorough, present clear trade-off analysis across four options, and include appropriate caveats about illustrative OpenShell configuration. However, the implementation plan contains a broken cross-reference to a non-existent ADR-0029, which must be corrected before merge.

Findings

High

  • [Correctness] docs/plans/agent-execution-environment.md:929 — The References section links to ADR-0029: Agent Execution Sandbox Architecture at path ../ADRs/0029-agent-execution-sandbox.md, but the actual ADR file added in this PR is 0036-agent-execution-sandbox.md. There is no ADR-0029 in the repository. This is a broken link that will confuse readers following cross-references.
    Remediation: Update the reference to [ADR-0036: Agent Execution Sandbox Architecture](../ADRs/0036-agent-execution-sandbox.md).

Medium

  • [Correctness] docs/ADRs/0036-agent-execution-sandbox.md:293 — ADR-0036 References section does not include ADR-0030 (OpenShell sandbox interaction model), despite the implementation plan and Dockerfile referencing ADR-0030 decisions. Adding it would improve traceability.
    Remediation: Add - ADR-0030: OpenShell sandbox interaction model to the References section of 0036-agent-execution-sandbox.md.

Low

  • [Style/conventions] README.md — The new docs/plans/ section bundles documenting two pre-existing plan files alongside the new agent-execution-environment plan. Minor commit hygiene note.

  • [Intent alignment] docs/plans/agent-execution-environment.md — ~930 lines of implementation detail for a Proposed-status ADR. Appropriate caveats are present but reviewers should note the volume of pre-acceptance detail.

Info

  • [Correctness] The PR branch name adr-0029-agent-sandbox suggests the ADR was originally numbered 0029 and was renumbered to 0036. All in-file references use 0036 correctly except the one broken link noted above.

  • [Injection defense] PR body, commit messages, and document content were inspected. No prompt injection patterns, non-rendering Unicode, bidirectional overrides, or suspicious directives found.

  • [Platform security] No code changes, no authentication/authorization logic, no credential handling in the diff. The documents discuss security architecture appropriately as design considerations without introducing actual security-sensitive code.

Footer

Outcome: request-changes
This review applies to SHA cb8a17fb54c78d64a7f541d6676b4146b504f3aa. Any push to the PR head clears this review and requires a new evaluation.

Previous run (2)

Review: #659

Head SHA: a6b6eb0
Timestamp: 2026-05-11T00:00:00Z
Outcome: comment-only

Summary

This is a well-structured documentation PR that adds ADR-0036 (Agent Execution Sandbox Architecture) and a companion implementation plan for the agent execution environment. The ADR properly addresses the agent execution question deferred in ADR-0028, presents clear trade-off analysis across four options, and the implementation plan is thorough. Two non-blocking findings: a broken cross-reference in the implementation doc's References section (links to ADR-0029 instead of ADR-0036, likely leftover from the original numbering matching the branch name), and an illustrative token that may trigger automated secret scanners.

Findings

Medium

  • [broken-link] docs/plans/agent-execution-environment.md:~922 — The References section links to ADR-0029: Agent Execution Sandbox Architecture at path ../ADRs/0029-agent-execution-sandbox.md, but the ADR file added in this PR is 0036-agent-execution-sandbox.md. This is a broken link. The branch name adr-0029-agent-sandbox suggests the ADR was originally numbered 0029 and later renumbered, but this reference wasn't updated.
    Remediation: Update to [ADR-0036: Agent Execution Sandbox Architecture](../ADRs/0036-agent-execution-sandbox.md).

Low

  • [secret-scanning] docs/plans/agent-execution-environment.md:~274 — The illustrative token ghp_realtoken123... in a YAML comment matches the ghp_ prefix pattern for GitHub personal access tokens. While clearly a documentation example, it may trigger automated secret scanning tools.
    Remediation: Consider using a more obviously fake token like ghp_EXAMPLE_TOKEN_NOT_REAL.

Footer

Outcome: comment-only
This review applies to SHA a6b6eb00ba5d00e86af875976488c4b6d60e39a7. Any push to the PR head clears this review and requires a new evaluation.

Previous run (3)

Review: #659

Head SHA: 7308582
Timestamp: 2026-05-09T00:00:00Z
Outcome: approve

Summary

This PR adds ADR-0029 (agent execution sandbox architecture) and an accompanying implementation plan document. The ADR is well-structured, presents four options with clear trade-offs, and makes a defensible decision for a shared Docker-first container image. The implementation document provides detailed, actionable guidance covering the container build pipeline, OpenShell configuration, resource limits, privileged container alternatives, GitLab runner setup, image signing, and upgrade procedures. All cross-references to existing ADRs (0005, 0017, 0025, 0028, 0030) and problem documents are valid. The README update correctly documents the previously-unlisted docs/plans/ directory. No correctness, security, or injection issues were found.

Findings

Info

  • [style] docs/ADRs/0029-agent-execution-sandbox.md:25 — The ADR frontmatter relates_to only lists agent-infrastructure, but the body substantively references ADR-0017, ADR-0025, ADR-0028, and ADR-0030. Consider adding these to the frontmatter for discoverability.

  • [correctness] docs/plans/agent-execution-environment.md:503 — The caveat noting that OpenShell configuration examples are illustrative and must follow ADR-0030's accepted decisions is well-placed. This is a positive observation — the document correctly separates design-phase exploration from the accepted interaction model.

  • [style] README.md — The PR adds a docs/plans/ section that includes two pre-existing files (drift scanner, vertex inference provisioning) alongside the new agent-execution-environment doc. This is reasonable scope — documenting a previously-unlisted directory — but is worth noting as beyond the strict ADR-0029 scope.

  • [correctness] docs/plans/agent-execution-environment.md:409 — Dockerfile ENTRYPOINT comment references "ADR-0030" which is confirmed to exist with status Accepted. Cross-reference is valid.

Footer

Outcome: approve
This review applies to SHA 73085820fc7c2dee6b5da0b9c382c5653a13f954. Any push to the PR head clears this review and requires a new evaluation.

Previous run (4)

Review: #659

Head SHA: 665de4b
Timestamp: 2026-05-09T00:00:00Z
Outcome: comment-only

Summary

This PR adds ADR-0029 (agent execution sandbox architecture) and a companion implementation plan document. The ADR is well-structured with clear options analysis, sound decision rationale favoring a shared container image approach, and honest treatment of consequences and risks. The implementation document provides thorough operational guidance for container builds, OpenShell configuration, resource limits, GitLab runner setup, image signing, and upgrade procedures. The caveat about ADR-0030 overriding illustrative OpenShell interaction details is a good practice that prevents stale design docs from being treated as implementation specs. A few minor observations are noted below but nothing that should block this PR.

Findings

Medium

  • [correctness] docs/plans/agent-execution-environment.md:~line 84 — The before_script block in the GitLab registry authentication example runs docker login inside the agent sandbox container, but the comment says it's for pulling additional images during agent execution. If agents are sandboxed with L7 network policies, they shouldn't have Docker socket access (the doc itself warns against mounting /var/run/docker.sock). This example may confuse implementers about what is and isn't permitted inside the sandbox.
    Remediation: Clarify that registry authentication for the job's base image is handled by the runner (via image_pull_secrets or runner config), and remove or rework the before_script Docker login example to avoid implying agents have Docker daemon access.

Low

  • [correctness] README.md — The PR adds a docs/plans/ section linking three documents, but only agent-execution-environment.md is introduced by this PR. The other two links (2026-03-06-adr46-drift-scanner.md, vertex-inference-provisioning.md) are pre-existing files that weren't previously linked in the README. This is helpful but expands scope beyond what the PR title and description suggest.
    Remediation: Consider noting in the PR description that the README update also adds links to pre-existing plan documents, or split that into a separate commit for clarity.

  • [style] docs/ADRs/0029-agent-execution-sandbox.md:~line 103 — The layer description says "Layer 3: Agent harness (fullsend run CLI)" and parenthetically explains what it provides. The other layers don't have such parenthetical explanations, creating inconsistency. Minor style nit.

Info

  • [correctness] docs/ADRs/0029-agent-execution-sandbox.md — The ADR references ADR-0028 which is still in "Proposed" status. The PR description correctly notes the dependency on PR Add ADR-0028 for GitLab support architecture #601 and merge ordering. No action needed, just confirming the dependency is real and documented.

  • [intent-alignment] The PR body states this depends on Add ADR-0028 for GitLab support architecture #601 (ADR-0028) and should be merged after it. This is a reasonable sequencing constraint since ADR-0029 builds on ADR-0028's decision to support GitLab.

  • [correctness] docs/plans/agent-execution-environment.md — The OpenShell configuration section includes a prominent caveat that ADR-0030 (Accepted) overrides the illustrative details. This is good practice — it prevents the illustrative API examples from being mistakenly treated as the implementation spec.

Footer

Outcome: comment-only
This review applies to SHA 665de4b43d8b7f91e7930e85141a3489a284b60f. Any push to the PR head clears this review and requires a new evaluation.

Previous run (5)

Review: #659

Head SHA: a53603d
Timestamp: 2026-05-09T00:00:00Z
Outcome: approve

Summary

This documentation-only PR adds ADR-0029 (agent execution sandbox architecture) and a companion implementation plan. The ADR is well-structured, presents four options with clear trade-offs, and makes a well-reasoned decision for a shared container image approach. All cross-references to existing ADRs (0017, 0025, 0028, 0030) and problem documents (agent-infrastructure.md) are valid. The implementation plan correctly disclaims that OpenShell configuration examples are illustrative and defers to ADR-0030 for the actual interaction model. The README addition appropriately adds a docs/plans/ section covering both pre-existing and new plan documents. No blocking concerns identified.

Findings

Medium

  • [correctness] docs/plans/agent-execution-environment.md:737-756 — The PodSecurityPolicy YAML example uses the policy/v1beta1 API which was removed in Kubernetes 1.25. While the doc acknowledges this and provides Pod Security Standards as the modern alternative, the deprecated PSP example could mislead implementors on older clusters into using a removed API.
    Remediation: Consider removing the PSP YAML or adding an explicit inline note that this API no longer exists in K8s 1.25+ (the current parenthetical is easy to miss).

Low

  • [correctness] docs/plans/agent-execution-environment.md:696-699 — The rootless Docker example uses sleep 5 to wait for dockerd-rootless.sh startup, which is a brittle race condition pattern. If the daemon takes longer to initialize, the subsequent docker run will fail.
    Remediation: Use a readiness check loop (e.g., until docker info; do sleep 1; done) instead of a fixed sleep.

  • [style/conventions] docs/ADRs/0029-agent-execution-sandbox.md:44 — ADR-0028 is referred to as decided ("With GitLab support decided (ADR-0028)") but ADR-0028's own front matter still has status: Proposed. Minor inconsistency.
    Remediation: Either soften the phrasing to "proposed" or coordinate with an ADR-0028 status update.

Info

  • [correctness] docs/plans/agent-execution-environment.md — OpenShell version 0.0.37-dev is referenced as a dev/pre-release version. Appropriate for a "Proposed" ADR but should be updated to a stable release before the ADR moves to "Accepted".

  • [style/conventions] README.md — The docs/plans/ section addition bundles housekeeping (linking two pre-existing plan docs) with the new content. This is fine and improves discoverability of existing documents.

Footer

Outcome: approve
This review applies to SHA a53603ddd391a7d50e1aef214a3b8864466cb5cb. Any push to the PR head clears this review and requires a new evaluation.

Previous run (6)

Review: #659

Head SHA: 0c06fd7
Timestamp: 2026-05-09T00:00:00Z
Outcome: comment-only

Summary

This PR adds ADR-0029 (agent execution sandbox architecture) and a companion implementation plan, along with a README update to surface the docs/plans/ directory. The documents are well-structured, present clear trade-off analysis across four options, and arrive at a reasonable decision (shared container image). The primary concern is that the implementation plan's description of OpenShell interaction mechanics diverges from the already-accepted ADR-0030 in several specifics — the REST API model, file delivery assumptions, and gateway invocation differ from ADR-0030's decided CLI+SSH+SCP model. The ADR's caveat note about illustrative examples partially mitigates this, but the documents would benefit from alignment with ADR-0030 to avoid confusing implementers.

Findings

Medium

  • [Correctness] docs/plans/agent-execution-environment.md:530-537 — The sandbox creation flow describes a REST API call (POST http://127.0.0.1:8080/v1/sandboxes) and iptables-based L7 enforcement. ADR-0030 (accepted) establishes that fullsend interacts with OpenShell via its CLI (openshell sandbox create), uses SSH for command execution, and the gateway's HTTP proxy layer handles L7 policy enforcement — not iptables. The gateway configuration YAML (/etc/openshell/gateway.yaml) and policy YAML format (endpoint/methods/binaries) are also speculative and may not match the actual OpenShell configuration model.
    Remediation: Align the sandbox creation flow and configuration examples with ADR-0030's decided interaction model (CLI-based sandbox creation, SSH tunneling, provider-based credential delivery via openshell provider create), or explicitly note which parts are aspirational vs. which follow ADR-0030.

  • [Correctness] docs/plans/agent-execution-environment.md:403-407 — The Dockerfile ENTRYPOINT uses ["openshell", "gateway"] as PID 1. ADR-0030 uses openshell gateway start which spawns a persistent process. Additionally, agent config files are assumed to be baked into the image at /opt/fullsend/agents/<agent-name>/config.yaml, but ADR-0030 decides that agent definitions and host files are SCP'd during bootstrap — only tool binaries and runtimes are baked into the container image.
    Remediation: Update the Dockerfile entrypoint to match ADR-0030's openshell gateway start invocation, and clarify which files are image-baked vs. bootstrap-delivered per ADR-0030's file delivery decision.

Low

  • [Correctness] docs/plans/agent-execution-environment.md:377-379 — The runtimes stage installs golang-1.23 via apt-get install, which is not a valid Ubuntu 22.04 package name (the package is typically golang-go or a PPA-specific name). The builder stage correctly uses the official golang:1.23 Docker image.
    Remediation: Use a valid package name or note that Go runtime installation requires a PPA or manual download rather than the default apt repositories.

Info

  • [Style] docs/ADRs/0029-agent-execution-sandbox.md:235 — The caveat note about illustrative OpenShell examples is well-placed and important. Consider making it more prominent (e.g., as a blockquote or admonition at the top of the implementation plan as well) since the plan contains extensive speculative configuration that readers might take as authoritative.

  • [Style] README.md — The new docs/plans/ section appropriately surfaces two pre-existing orphaned files alongside the new one. Good housekeeping.

Footer

Outcome: comment-only
This review applies to SHA 0c06fd7cfa143b2395c5b4e0a22cd708118d09ad. Any push to the PR head clears this review and requires a new evaluation.

Previous run (7)

Review: #659

Head SHA: 27409bf
Timestamp: 2026-05-09T00:00:00Z
Outcome: approve

Summary

This PR adds ADR-0029 (agent execution sandbox architecture) and a companion implementation plan for the agent execution environment. The change is well-scoped: it addresses the execution environment question explicitly deferred by ADR-0028 (GitLab support). The ADR follows the repo's established format, presents four options with clear trade-offs, and the decision rationale (shared container image, Docker-first) is well-argued from a security-parity standpoint. The implementation plan provides thorough guidance across build pipeline, OpenShell configuration, resource limits, signing, and cross-platform considerations. The README update correctly adds the docs/plans/ section with links to all three existing plan documents. No critical or high findings.

Findings

Medium

  • [correctness] docs/plans/agent-execution-environment.md:829 — The Kubernetes executor Helm install example uses runnerRegistrationToken which the same document (line 780) notes was deprecated in GitLab 15.10 and removed in 17.0. The Docker executor section includes a clear deprecation note with the recommended --token alternative, but the Kubernetes/Helm section does not include an equivalent note or updated syntax.
    Remediation: Add a deprecation note to the Helm install section similar to the Docker executor section, and show the updated Helm values using runnerToken instead of runnerRegistrationToken.

  • [correctness] docs/plans/agent-execution-environment.md:732-750 — The PodSecurityPolicy example uses apiVersion: policy/v1beta1 which is deprecated in Kubernetes 1.21 and removed in 1.25. The document does note this inline (deprecated in K8s 1.25, removed in 1.29) but the removal version is incorrect — PSP was removed in 1.25, not 1.29.
    Remediation: Update the inline comment to deprecated in K8s 1.21, removed in 1.25 to match the actual Kubernetes deprecation timeline.

Low

  • [style] docs/ADRs/0029-agent-execution-sandbox.md:103 — Minor: parenthetical about Fedora-based images and fullsend run compatibility is a tangential detail that slightly interrupts the flow of the image composition strategy description. Consider moving to the Open Questions section or a footnote.

Info

  • [intent-alignment] The PR depends on Add ADR-0028 for GitLab support architecture #601 (ADR-0028) which is still in Proposed status. ADR-0029 references ADR-0028 as a decided fact. This is fine for design exploration, but the dependency should be tracked — if ADR-0028 is rejected, ADR-0029's GitLab-related sections would need revision.

  • [correctness] docs/plans/agent-execution-environment.md:860-863 — The before_script runs docker login inside the agent container, but agents run inside OpenShell sandboxes with restricted network access. The comment explains this is for pulling additional images, but in practice the sandbox L7 policies would need to allow registry access for this to work. This is noted for implementation-time consideration.

Footer

Outcome: approve
This review applies to SHA 27409bf4dba35b8d79f048bd5e9afcbd74fe3ccc. Any push to the PR head clears this review and requires a new evaluation.

Previous run (8)

Review: #659

Head SHA: dc59975
Timestamp: 2026-05-09T00:00:00Z
Outcome: comment-only

Summary

This is a well-structured documentation PR adding ADR-0029 (agent execution sandbox architecture) and a companion implementation plan. The ADR follows established conventions, presents four options with clear trade-offs, and makes a well-reasoned decision for a shared container image approach. The dependency on PR #601 (ADR-0028) is satisfied — that PR is merged. No code changes are included, so platform security, content security, and correctness dimensions are not materially implicated. Two minor style/completeness findings are noted below.

Findings

Medium

None

Low

  • [incomplete-index] README.md:42-43 — The new docs/plans/ section in the README lists only the newly added agent-execution-environment.md, but the docs/plans/ directory already contains two other files on main (2026-03-06-adr46-drift-scanner.md and vertex-inference-provisioning.md). This creates an incomplete index. Consider listing the existing plans as well, or adding them in a follow-up PR.

  • [stale-api-reference] docs/plans/agent-execution-environment.md — The GitLab runner registration example uses --registration-token, which GitLab deprecated in favor of glrt- runner authentication tokens (GitLab 15.10+, fully removed in 17.0). Since this is a design doc with illustrative examples rather than production config, this is low severity, but worth a note in the doc or an update to the new registration flow.

Info

  • [deprecated-k8s-api] docs/plans/agent-execution-environment.md:413-432 — The PodSecurityPolicy example correctly notes PSP is deprecated in K8s 1.25 and removed in 1.29. The doc also includes the Pod Security Standards replacement, which is good. The PSP example could be removed or moved to a "legacy" callout to avoid confusion, but this is informational since the doc already flags the deprecation.

  • [lint-not-verified]make lint could not be run due to network restrictions in the review sandbox. The PR should pass lint in CI before merging, per CLAUDE.md instructions.

Footer

Outcome: comment-only
This review applies to SHA dc599755b19a8cc9d8a42709aa7adfa4fc40a115. Any push to the PR head clears this review and requires a new evaluation.

Previous run (9)

Review: #659

Head SHA: 6c94c8a
Timestamp: 2026-05-09T00:00:00Z
Outcome: comment-only

Summary

This PR adds ADR-0029 (agent execution sandbox architecture) and a companion implementation plan document. The design is well-structured, explores four options with honest trade-offs, and makes a clear decision for the shared container image approach. The security discussion (privileged container requirements, rootless alternatives, image signing) is thorough. No critical or high findings. Two medium findings relate to README categorization and a stale ADR-0028 reference.

Findings

Medium

  • [style/conventions] README.md:36 — The new link to docs/plans/agent-execution-environment.md is placed inside the docs/problems/ listing section ("Deep dives into each major problem domain, each evolving independently"). The document lives in docs/plans/, not docs/problems/, creating a categorization mismatch. Either move the document to docs/problems/, add a separate docs/plans/ section to the README index, or place the link under a different heading.
    Remediation: Add a docs/plans/ section to the README (e.g., between the problems list and the guides entry), or move the document into docs/problems/ if it fits that pattern.

  • [correctness] docs/plans/agent-execution-environment.md:920 — The references section states "ADR-0028: GitLab Support Architecture (pending in PR Add ADR-0028 for GitLab support architecture #601)" but PR Add ADR-0028 for GitLab support architecture #601 is already merged and ADR-0028 exists in the repo. This reference is stale.
    Remediation: Update to a direct link: [ADR-0028: GitLab Support Architecture](../ADRs/0028-gitlab-support.md)

Low

Info

  • [correctness] docs/plans/agent-execution-environment.md:728 — The PodSecurityPolicy YAML example is for an API (policy/v1beta1) that was removed in Kubernetes 1.29. The document does note this in a comment, and provides the Pod Security Standards alternative — just flagging that the PSP example may confuse readers on modern clusters. The existing inline comment is sufficient.

Footer

Outcome: comment-only
This review applies to SHA 6c94c8ac4d7ee34949dc06bfa57e8c9bb9b57a2f. Any push to the PR head clears this review and requires a new evaluation.

Previous run (10)

Review: #659

Head SHA: 2fe5274
Timestamp: 2026-05-05T00:00:00Z
Outcome: comment-only

Summary

This is a well-structured documentation PR that adds ADR-0029 (agent execution sandbox architecture) and a companion implementation document. The ADR follows the repo's template, considers four options with clear trade-offs, and makes a defensible decision for a shared container image approach. The security implications of privileged containers are thoroughly documented with mitigations. Two medium-severity observations are noted below regarding ADR numbering dependency and an unverifiable external reference, neither of which block the PR.

Findings

Medium

  • [intent-alignment] docs/ADRs/0029-agent-execution-sandbox.md — This ADR depends on ADR-0028 (PR Add ADR-0028 for GitLab support architecture #601), which has not merged yet. ADR-0027 is the latest on main. If any other ADR merges as 0028 or 0029 before these two PRs land, there will be numbering conflicts. The PR description correctly notes the merge ordering dependency, and the repo has a renumber-adr skill for this purpose, but reviewers should ensure PR Add ADR-0028 for GitLab support architecture #601 merges first or renumbering is applied before merge.

  • [correctness] docs/ADRs/0029-agent-execution-sandbox.md / docs/problems/agent-execution-environment.md — The documents reference "OpenShell" from NVIDIA (https://github.com/NVIDIA/OpenShell) with specific version numbers (0.0.37-dev), API endpoints, configuration syntax, and behavioral details (PID 1 gateway, L7 proxy on port 3128, /proc/<pid>/exe binary enforcement). These details read as authoritative but could not be independently verified against a public project. If OpenShell's actual API or configuration format differs, the implementation guidance would be misleading. Consider adding a note that configuration examples are illustrative and subject to validation against the actual OpenShell release used.

Low

  • [style/conventions] docs/problems/agent-execution-environment.md — The document is 920 lines, which is substantially longer than most problem documents in docs/problems/. This is reasonable given the implementation detail scope, but some sections (e.g., GitLab runner configuration, Kubernetes PodSecurityPolicy YAML) are operational runbooks rather than problem exploration. Consider whether some of this content belongs in docs/guides/ per ADR-0023 (user documentation structure) once the design stabilizes.

Info

  • [correctness] README.md — The new entry is correctly inserted in alphabetical order between "Testing the Agents" and "Operational Observability", maintaining the existing list structure.

  • [platform-security] docs/ADRs/0029-agent-execution-sandbox.md — The ADR correctly identifies the privileged container requirement as the primary security trade-off and documents three mitigation paths (rootless Podman, user namespace remapping, eBPF-based enforcement). The defense-in-depth approach for the host-side REST server (per-run bearer token even on localhost) is a good security practice.

  • [injection-defense] No injection patterns, suspicious Unicode, or instruction-like directives detected in the PR body, commit messages, or document content.

Footer

Outcome: comment-only
This review applies to SHA 2fe52747c8af98cde7f6314682d0dc4711356e87. Any push to the PR head clears this review and requires a new evaluation.

Previous run (11)

Review: #659

Head SHA: 1c3ecb8
Timestamp: 2026-05-05T00:00:00Z
Outcome: comment-only

Summary

This is a well-structured documentation PR adding ADR-0029 (agent execution sandbox architecture) and a companion implementation document. The ADR thoroughly evaluates four options, justifies the shared-container-image decision with clear security rationale, and documents trade-offs honestly. The implementation document provides comprehensive guidance across container builds, OpenShell configuration, resource limits, image signing, and platform-specific considerations. One medium finding: the implementation document contains a markdown link to ../ADRs/0028-gitlab-support.md which does not exist on the base branch — it depends on PR #601 merging first, creating a broken link if this PR lands first.

Findings

Medium

Info

  • [design-quality] The ADR decision rationale is strong — the security argument for environment parity across GitHub/GitLab as a requirement (not just convenience) is well-articulated. The rejection of Option 4 (dynamic tool installation) correctly identifies the zero-trust violation. The privileged container trade-off is documented honestly with mitigations and open questions for rootless alternatives.

  • [deprecation-note] docs/problems/agent-execution-environment.md:727-745 — The PodSecurityPolicy example uses the policy/v1beta1 API which was removed in Kubernetes 1.29. The document correctly notes this deprecation and provides the Pod Security Standards replacement immediately after, so no action needed — just noting for awareness.

Footer

Outcome: comment-only
This review applies to SHA 1c3ecb893adce823a16fe440598976cc2e02c427. Any push to the PR head clears this review and requires a new evaluation.

Previous run (12)

Review: #659

Head SHA: 8c27c94
Timestamp: 2026-05-05T00:00:00Z
Outcome: comment-only

Summary

This is a well-structured documentation PR adding ADR-0029 (Agent Execution Sandbox Architecture) and a companion implementation document (docs/problems/agent-execution-environment.md), with a README index entry. The ADR thoroughly evaluates four options (shared container image, platform-specific images, Kubernetes CRDs, minimal sandbox + dynamic tools), clearly justifies the decision for shared Docker-first container image, and honestly documents consequences and open questions. Security analysis is strong — the privileged container trade-off is well-documented with mitigations, image signing via Sigstore is detailed, and the explicit warning against Docker socket mounting is important. The main concern is a merge-order dependency: this PR extensively references ADR-0028 (GitLab support from PR #601), which has not yet merged and does not exist on main.

Findings

Medium

Low

  • [style/conventions] docs/problems/agent-execution-environment.md — The document is placed in docs/problems/ but opens with "This document contains implementation details for the agent execution sandbox architecture." CLAUDE.md states problem documents should "present multiple options with trade-offs, not prescribe single solutions." The ADR correctly presents options, but this 920-line companion document is primarily an implementation specification (Dockerfile, CI/CD YAML, runner configs, TOML snippets) for the chosen option rather than a problem exploration. This follows the same pattern as gitlab-implementation.md from PR Add ADR-0028 for GitLab support architecture #601 and may be intentional, but it is a departure from the problem-doc convention used by the other 18 documents in docs/problems/.
    Remediation: No action required if this is an intentional convention evolution for ADR companion docs. Consider adding a brief note at the top (e.g., "This is an implementation companion to ADR-0029, not a problem exploration") to set reader expectations, or relocate to a docs/implementation/ directory if more implementation docs follow this pattern.

  • [correctness] docs/problems/agent-execution-environment.md:727-745 — The PodSecurityPolicy example uses the deprecated policy/v1beta1 API, removed in Kubernetes 1.29. The document correctly notes this ("deprecated in K8s 1.25, removed in 1.29") and provides the Pod Security Standards alternative immediately after, so the risk of copy-paste error is low. However, leading with the deprecated API may cause confusion for readers on modern clusters.
    Remediation: Consider reordering to present Pod Security Standards first (current recommended approach) and the PSP example second as a legacy reference, or remove the PSP YAML entirely since it targets a removed API.

Info

  • [correctness] ADR numbering is correct: 0029 follows the latest on main (0027), leaving room for 0028 from PR Add ADR-0028 for GitLab support architecture #601. No numbering collision.

  • [intent alignment] PR scope matches stated intent — addresses the agent execution environment question explicitly deferred in PR Add ADR-0028 for GitLab support architecture #601. Documentation-only change with no code modifications. "Proposed" status is appropriate for a design exploration seeking feedback.

  • [content security] No secrets, API keys, or sensitive identifiers found. All examples use placeholder values ($GITHUB_TOKEN, $REGISTRATION_TOKEN, ghp_realtoken123... clearly marked as illustrative).

  • [injection defense] No prompt injection patterns, non-rendering Unicode, or bidirectional overrides detected in PR body, commit messages, or document content.

Footer

Outcome: comment-only
This review applies to SHA 8c27c9439204f7bc1d7cb31824fab065e583e94d. Any push to the PR head clears this review and requires a new evaluation.

Previous run (13)

Review: #659

Head SHA: a94832b
Timestamp: 2026-05-05T00:00:00Z
Outcome: comment-only

Summary

This PR adds ADR-0029 (agent execution sandbox architecture) and a companion implementation document for the agent execution environment. The documents are well-structured, present multiple options with trade-offs, and follow the repo's design exploration conventions. The decision rationale is sound — a shared container image provides environment parity across GitHub Actions and GitLab CI, which is a genuine security requirement. Several findings are worth noting: the example Dockerfile references an undefined build stage, the GitLab runner setup example mounts the Docker socket (contradicting the security isolation goals), and the ADR references documents (ADR-0028, gitlab-implementation.md) that don't yet exist in the repo. None of these block the PR — they are design document quality issues that should be addressed before the ADR moves past "Proposed" status.

Findings

Medium

  • [Correctness] docs/problems/agent-execution-environment.md (Dockerfile example, ~line 75 in the example) — The Dockerfile uses COPY --from=builder /app/fullsend /usr/local/bin/fullsend but no builder stage is defined in the multi-stage build. The stages defined are base, runtimes, tools, harness, and final. As written, this Dockerfile would fail to build.
    Remediation: Either add a FROM golang:1.23 AS builder stage that compiles the fullsend binary, or add a comment indicating the builder stage is defined in a separate file/context.

  • [Platform Security] docs/problems/agent-execution-environment.md (GitLab Runner registration command) — The runner registration example includes --docker-volumes "/var/run/docker.sock:/var/run/docker.sock", which mounts the host Docker socket into the agent container. This gives the container (and any agent running inside it) full control over the host Docker daemon, effectively bypassing all sandbox isolation. This contradicts the document's own security model. In a privileged container with Docker socket access, an agent could start arbitrary containers on the host, access other containers' filesystems, or escape the sandbox entirely.
    Remediation: Remove the Docker socket mount from the runner registration example. If Docker socket access is needed for a specific use case (e.g., Docker-in-Docker), document it separately with appropriate warnings, or reference the builder services section which already addresses this concern.

  • [Correctness] docs/ADRs/0029-agent-execution-sandbox.md — Multiple references to ADR-0028 and gitlab-implementation.md that do not exist in the repository. The highest existing ADR is 0027. While ADR-0028 may be introduced by PR Add ADR-0028 for GitLab support architecture #601, readers following these links will get broken references. This also creates an ADR numbering gap (0027 → 0029) if ADR-0028 is not merged first.
    Remediation: Either merge PR Add ADR-0028 for GitLab support architecture #601 (ADR-0028) before this PR, or add a note that ADR-0028 is pending in PR Add ADR-0028 for GitLab support architecture #601. Consider whether this ADR should be numbered 0028 instead if the GitLab ADR hasn't been accepted yet.

Low

  • [Correctness] docs/ADRs/0029-agent-execution-sandbox.md:270 — Placeholder issue link NVIDIA/OpenShell#xyz with comment "actual issue TBD". Should either be filed and linked, or removed if the upstream feature request hasn't been created yet.
    Remediation: Replace with an actual issue link or remove the placeholder link and just describe the feature request.

  • [Correctness] docs/problems/agent-execution-environment.md (GitHub Actions workflow example) — Uses docker/run@v1 which is not a standard GitHub Action. The Docker organization publishes docker/build-push-action, docker/setup-buildx-action, etc., but there is no official docker/run action.
    Remediation: Replace with a standard run: step using docker run directly (as shown correctly in the Platform-Specific Considerations section later in the same document).

  • [Correctness] docs/problems/agent-execution-environment.md (GitLab registry auth example) — The before_script runs docker login inside a job that's already using image:, meaning the executor has already pulled the image. The docker login would only be useful for pulling additional images during the job, but the example doesn't explain this.
    Remediation: Add a comment clarifying the purpose, or restructure the example to show registry auth at the runner level (where it's actually needed for image pulls).

Info

  • [Style] docs/problems/agent-execution-environment.md — PodSecurityPolicy example uses the deprecated policy/v1beta1 API (removed in Kubernetes 1.29). The document acknowledges this inline, and the Pod Security Standards alternative is provided immediately after. No action needed, but consider removing the deprecated example in a future revision to avoid confusion.

  • [Style] Both documents are thorough and well-organized. The separation of ADR (decision rationale) from implementation document (detailed guidance) follows good practice. The open questions sections are appropriately scoped.

Footer

Outcome: comment-only
This review applies to SHA a94832b9bf6fbafaf2a68fcc283d04c25bebd2ea. Any push to the PR head clears this review and requires a new evaluation.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

Comment thread docs/ADRs/0029-agent-execution-sandbox.md Outdated
Comment thread docs/ADRs/0036-agent-execution-sandbox.md
Comment thread docs/ADRs/0036-agent-execution-sandbox.md
ggallen and others added 5 commits May 9, 2026 15:44
Addresses the open question from GitLab implementation (PR fullsend-ai#601) about how
agents execute on GitLab runners vs GitHub Actions.

Key decisions:
- Shared container image (ghcr.io/fullsend-ai/agent-sandbox) for both GitHub
  Actions and GitLab CI
- OpenShell as PID 1 with nested sandboxes for individual agents
- Docker and Kubernetes executors supported (shell executor excluded)
- Privileged container requirement for OpenShell network namespace manipulation
- Resource limits and timeout enforcement per platform
- Image signing with Sigstore for supply chain integrity

The ADR explores four options:
1. Shared container image, Docker-first (chosen)
2. Platform-specific images (rejected: maintenance burden, inconsistent security)
3. Kubernetes-native with CRDs (rejected: poor fit for ephemeral task execution)
4. Minimal sandbox + dynamic tools (rejected: violates zero-trust execution)

Implementation details moved to docs/problems/agent-execution-environment.md:
- Container image build pipeline and Dockerfile structure
- OpenShell gateway configuration and sandbox creation flow
- L7 policy and provider configuration examples
- Platform-specific considerations (GitHub Actions VMs, GitLab Docker/K8s executors)
- Host-side REST server lifecycle in containerized environments
- Image signing, verification, upgrade, and rollback procedures

Open questions documented:
- Rootless OpenShell support (user namespaces, eBPF alternatives)
- Image build and distribution strategy (public vs per-org registry)
- Builder services for Docker-in-Docker use cases (external Kaniko vs prohibit)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Lint fixes:
- Remove gitlab-implementation from ADR frontmatter relates_to (doesn't exist yet)
- Update references to ADR-0028 and gitlab-implementation.md to note they are pending in PR fullsend-ai#601

Review findings (Medium):
- Add builder stage to Dockerfile example (was missing, would fail to build)
- Remove Docker socket mount from GitLab runner registration (security violation - bypasses sandbox isolation)
- Update all references to pending GitLab support work (PR fullsend-ai#601 instead of ADR-0028)

Review findings (Low):
- Remove placeholder OpenShell issue link, describe as upstream feature request instead
- Replace non-existent docker/run@v1 action with standard docker run command
- Add comment explaining docker login in GitLab job (for pulling additional images, not the base image)

All changes address findings from fullsend-ai-review comment-only review.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace markdown link to non-existent ADR-0028 file with plain text noting
it is pending in PR fullsend-ai#601. This prevents broken link if this PR merges before
fullsend-ai#601.

Addresses review finding about broken link in References section.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
**From @maruiz93:**
- Add note about Fedora/Podman compatibility consideration in image composition
- Clarify language runtimes are for fullsend's built-in agents; BYOA can customize
- Clarify harness role as control plane for agent execution
- Add disadvantage note about OpenShell feature parity validation needed for Kubernetes vs Docker

**From fullsend-ai-review bot:**
- Add note that OpenShell configuration examples are illustrative and subject to validation
- Enhance implementation doc header to clarify it's an ADR companion, not a problem exploration

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@ggallen
ggallen force-pushed the adr-0029-agent-sandbox branch from 2fe5274 to 73604fd Compare May 9, 2026 19:46
@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

fullsend review is working on this — view logs

This implementation document is a companion to ADR-0029, not a problem
exploration. Moving it to docs/plans/ better reflects its purpose as
implementation guidance.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

fullsend review is working on this — view logs

@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

Site preview

Preview: https://c5654e2a-site.fullsend-ai.workers.dev

Commit: 8a5c3e43fc3e8936a9341505a19259e9f8268c45

**Medium:**
- Add docs/plans/ section to README for proper categorization
- Update stale ADR-0028 reference in implementation doc (was "pending in PR fullsend-ai#601")

**Low:**
- Update ADR context to reflect GitLab support is decided (ADR-0028), not "under development"
- Update all references from "PR fullsend-ai#601" to ADR-0028 throughout

**Other:**
- Remove agent-execution-environment from ADR frontmatter relates_to (now in plans, not problems)
- Fix agent-infrastructure.md link path in implementation doc references

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

fullsend review is working on this — view logs

**Low:**
- Complete README docs/plans/ index with all three existing plan files
- Update GitLab runner registration to use modern authentication tokens (glrt-) instead of deprecated registration tokens, with note about GitLab 15.10+ requirement

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

fullsend review is working on this — view logs

**Medium:**
- Update Kubernetes Helm install to use modern runnerToken instead of deprecated runnerRegistrationToken
- Fix PodSecurityPolicy deprecation timeline (deprecated in 1.21, removed in 1.25, not 1.29)

**Low:**
- Move Fedora/Podman note from image composition to Rootless OpenShell Support section for better flow

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

fullsend review is working on this — view logs

**Medium:**
- Remove confusing docker login example from GitLab CI config
- Clarify that base image pull is handled by runner, not by agents in sandbox
- Agents do NOT have Docker daemon access (isolated by OpenShell)

**Low:**
- Remove parenthetical explanation from Layer 3 for consistency with other layers

Iteration 7/7 complete.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown

fullsend review is working on this — view logs

Comment thread docs/ADRs/0036-agent-execution-sandbox.md Outdated
@ifireball

Copy link
Copy Markdown
Member

@ggallen can you change the ADR number please? We already have ADR 29 in flight for the token mint, and a number of other follow-up ADRs that reference it.

Update agent execution sandbox ADR number from 0029 to 0036 to avoid conflicts with ADRs merged to main. Update all references in agent-execution-environment.md.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@ggallen ggallen changed the title Add ADR-0029 for agent execution sandbox architecture Add agent execution sandbox architecture ADR May 11, 2026
@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@ggallen

ggallen commented May 11, 2026

Copy link
Copy Markdown
Member Author

@ggallen can you change the ADR number please? We already have ADR 29 in flight for the token mint, and a number of other follow-up ADRs that reference it.

Just updated to 0036, which I think is the first free number.

- Mention Fedora/RHEL as base image alternatives for Podman deployments
- Add explicit note about BYOA image customization in Decision section
- Clarify GitLab CI script: field usage vs GitHub Actions
- OpenShell K8s/Docker parity already addressed in Option 3 disadvantages

Addresses review comments from maruiz93 and ifireball.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@ggallen ggallen left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Addressed all review feedback:

Re: Fedora mention (line 81): Added Fedora/RHEL as base image alternatives for Podman deployments in both Options and Decision sections.

Re: BYOA and language runtimes (line 42): Added explicit note in Decision section clarifying that organizations implementing custom agents can build customized images with different runtime sets.

Re: OpenShell features on Kubernetes (line 112): This concern was already addressed in the disadvantages list - line 146 notes that OpenShell feature parity between Kubernetes pod networking and Docker needs validation.

Re: GitLab script field (line 61): Added inline comment clarifying that GitLab CI uses the script: field for commands to run inside the container, while GitHub Actions uses the uses: action syntax.

@ggallen

ggallen commented May 11, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

- Fix broken link in agent-execution-environment.md: ADR-0029 → ADR-0036
- Add ADR-0030 reference to ADR-0036 References section

Addresses High and Medium severity findings from fullsend-ai-review.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

fullsend review is working on this — view logs

@ggallen
ggallen added this pull request to the merge queue May 11, 2026
Merged via the queue into fullsend-ai:main with commit 47e7e2d May 11, 2026
25 checks passed
@ggallen
ggallen deleted the adr-0029-agent-sandbox branch May 11, 2026 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants