docs: add guide for running agents locally with released binaries - #665
Conversation
ReviewFindingsLow
Previous runReviewFindingsLow
Previous run (2)ReviewFindingsLow
Previous run (3)ReviewFindingsLow
Info
Previous run (4)Review: #665Head SHA: 6783f21 SummaryClean documentation-only PR adding a user guide for running fullsend agents locally with released binaries (no Go toolchain required). The new guide at FindingsMedium
FooterOutcome: approve |
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
Thanks for writing this guide — local agent execution is a key gap and this addresses it well. Two items need changes before merging, plus a couple of notes for follow-up.
Also note: this PR should merge after #662 lands, since it documents behavior (cross-platform binary resolution, FULLSEND_SANDBOX_ARCH) that only exists on that branch. Consider linking issue #457 in the PR description — it directly describes this use case.
|
|
||
| ## Platform notes | ||
|
|
||
| ### macOS |
There was a problem hiding this comment.
[important] The prerequisites say Podman is required with OpenShell 0.0.37-dev+, but this section casually references Docker as if it works ("if using Docker instead of Podman..."). This sends mixed signals — a user with only Docker would think it's supported based on this note, but the prerequisites say otherwise.
Please either:
- Remove the Docker reference and state explicitly that Podman is the only supported runtime, or
- If Docker genuinely works, list it as an alternative in the prerequisites table.
Also, the guide never explains why OpenShell 0.0.37-dev+ is required (it's the version that adds Podman support). A sentence explaining this would help users understand whether they can use an older version with Docker.
There was a problem hiding this comment.
Fixed — the Docker reference was removed in an earlier update. The guide now exclusively references Podman as the container runtime (prerequisites table, gateway setup, platform notes, troubleshooting). No Docker mentions remain.
|
|
||
| > For building fullsend from source or contributing to the CLI, see [Local development](../dev/local-dev.md). | ||
|
|
||
| ## Prerequisites |
There was a problem hiding this comment.
[moderate] OpenShell is listed as a prerequisite but the guide never explains what it is, where to get it, or how to install it. The troubleshooting section mentions openshell version but a new user encountering OpenShell for the first time would be stuck. Please add a brief install step or link to OpenShell's installation docs.
There was a problem hiding this comment.
Fixed — added a dedicated "Step 2: Install OpenShell" section with uv tool install instructions, version verification, and gateway binary download from GitHub releases with a direct link.
|
|
||
| ```bash | ||
| gh repo clone <org>/.fullsend /tmp/fullsend-dot | ||
| ``` |
There was a problem hiding this comment.
[moderate — deferred] Nit: /tmp/fullsend.env is world-readable on most systems, which means tokens and credential paths are accessible to any local user. Consider suggesting ~/.config/fullsend/env as the example path and adding a chmod 600 note. Not blocking, but worth addressing in a follow-up.
There was a problem hiding this comment.
Acknowledged — will address in a follow-up. The sa-key.json already has a chmod 600 step, but the env file path could be improved.
| @@ -0,0 +1,197 @@ | |||
| # Running agents locally | |||
There was a problem hiding this comment.
[minor — deferred] This guide documents FULLSEND_SANDBOX_ARCH and cross-platform binary resolution behavior that only exists in PR #662's branch. This creates a merge ordering dependency — please merge after #662 lands. Also consider linking issue #457 ("Support local execution of individual agents for pre-adoption evaluation") in the PR description since it directly describes this use case.
There was a problem hiding this comment.
Acknowledged — this PR should merge after #662 lands. The cross-platform binary resolution and FULLSEND_SANDBOX_ARCH are documented based on that PR's implementation.
|
|
||
| ```bash | ||
| tar xzf fullsend_0.4.0_darwin_arm64.tar.gz | ||
| sudo mv fullsend_0.4.0_darwin_arm64/fullsend /usr/local/bin/ |
There was a problem hiding this comment.
Move it to $HOME/.local/bin instead.
There was a problem hiding this comment.
Done — updated to $HOME/.local/bin/ for both the fullsend binary and the openshell-gateway binary. No sudo needed.
| --no-post-script | ||
| ``` | ||
|
|
||
| The agent runs normally inside the sandbox, but the post-script is skipped. This means `PUSH_TOKEN` and `REVIEW_TOKEN` are not needed in your env file — only the core variables (`GH_TOKEN`, GCP credentials) are required. |
There was a problem hiding this comment.
Yes — verified against the code. PUSH_TOKEN and REVIEW_TOKEN are in the harness runner_env, which is only consumed by the post-script runner. With --no-post-script, the post-script is skipped entirely, so those variables are never read. Only GH_TOKEN (used by sandbox env files for repo cloning) and GCP credentials are needed.
|
Need details on
|
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
Review Summary
This is a well-structured documentation PR that addresses issue #457. The guide is comprehensive and follows good documentation practices.
Strategic assessment: ✅ Good direction. The change fits the project scope and directly addresses a documented user need.
Existing discussion: The PR already has thorough review comments from @ralphbean and @rh-hemartin covering the most important issues:
- Docker/Podman support clarity (important)
- OpenShell installation instructions (moderate)
- Security concerns with
/tmp/paths (moderate, deferred) - Merge ordering dependency on #662 (minor, deferred)
- Install path preferences and example improvements
I'm adding two minor notes below for future consideration. Approving since the documentation is solid and the existing comments cover the substantive issues.
Dependencies: This PR documents features from #662, so it should merge after #662 lands (already noted by @ralphbean).
| | Linux (x86_64) | `fullsend_{version}_linux_amd64.tar.gz` | | ||
| | Linux (arm64) | `fullsend_{version}_linux_arm64.tar.gz` | | ||
|
|
||
| Extract and move to a directory in your PATH: |
There was a problem hiding this comment.
[moderate — deferred] Consider adding a step to verify the downloaded binary checksum or signature. This is a security best practice for software distribution. GoReleaser can generate checksums files automatically — if the release includes a checksums.txt, add a verification step:
# Verify the checksum (optional but recommended)
shasum -a 256 -c checksums.txt 2>&1 | grep fullsend_0.4.0_darwin_arm64.tar.gzNot blocking for this PR, but worth considering for a future update.
There was a problem hiding this comment.
Acknowledged — will add checksum verification in a follow-up once we confirm GoReleaser is generating checksums.txt in the release artifacts.
| - The default `:latest` tag is amd64-only. Add `FULLSEND_SANDBOX_IMAGE=ghcr.io/fullsend-ai/fullsend-sandbox:dev` to your env file | ||
|
|
||
| **`unable to replace "host-gateway"` on macOS** | ||
| - Set `host_containers_internal_ip = "192.168.127.254"` under `[containers]` in `~/.config/containers/containers.conf` and restart the Podman machine |
There was a problem hiding this comment.
[minor — deferred] Consider adding a "Cleanup" or "Maintenance" section covering:
- How to remove stopped sandbox containers (
podman ps -a,podman rm) - When/how to update the local
.fullsendconfig directory clone - How to clean up temporary artifacts
Users following this guide might accumulate containers or wonder about keeping their config up-to-date. Not urgent, but would round out the guide.
There was a problem hiding this comment.
Acknowledged — cleanup/maintenance section is a good addition for a follow-up. OpenShell may already handle container cleanup, but documenting podman ps -a / podman rm and config repo updates would help.
New user guide covering the full workflow without a Go toolchain: download CLI from GitHub Releases, configure env, run agents on macOS (Podman Desktop) and Linux (Podman). Signed-off-by: Wayne Sun <gsun@redhat.com>
Explain how to run agent inference without post-script side effects (PR comments, branch pushes). Notes that PUSH_TOKEN/REVIEW_TOKEN are not needed when using this flag. Signed-off-by: Wayne Sun <gsun@redhat.com>
Align with PR #919 (migrate to Podman driver): - Bump OpenShell version from 0.0.37-dev+ to 0.0.38 - Add step 2: install OpenShell CLI and gateway binary - Add step 5: manually start and register the gateway - Remove Docker reference, add rootless Podman setup - Fix openshell version command and add gateway check Signed-off-by: Wayne Sun <gsun@redhat.com>
6783f21 to
91c8ccd
Compare
Site previewPreview: https://0230803a-site.fullsend-ai.workers.dev Commit: |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| | Requirement | macOS | Linux | | ||
| |-------------|-------|-------| | ||
| | Container runtime | Podman Desktop with a running machine | Podman | | ||
| | [OpenShell](https://github.com/NVIDIA/OpenShell) | 0.0.38 | 0.0.38 | |
There was a problem hiding this comment.
[low] documentation-consistency
OpenShell version listed as 0.0.38, while docs/guides/dev/local-dev.md lists 0.0.37-dev+. Version discrepancy between the two guides could confuse users.
Suggested fix: Align both guides to the current recommended OpenShell version.
fullsend-ai-review
Bot
added
the
ready-for-merge
- Fix health check port: 8081 (not 8080, which is the API port) - Add required env vars: OPENSHELL_SSH_HANDSHAKE_SECRET and OPENSHELL_SUPERVISOR_IMAGE (pinned to v0.0.38 SHA) - Add --health-port 8081 flag to gateway startup - Fix per-agent env vars: remove REPO_FULL_NAME from triage, add REVIEW_BODY_FILE for fix agent - Add gateway-specific troubleshooting entry Signed-off-by: Wayne Sun <gsun@redhat.com>
| | `REVIEW_BODY_FILE` | fix | Path to a file containing the review body to fix | | ||
|
|
||
| See the harness definitions in `harness/*.yaml` within your `.fullsend` config directory for the complete list per agent. | ||
|
|
There was a problem hiding this comment.
[low] correctness
REPO_FULL_NAME agent list (code, review, fix) differs from local-dev.md (triage, code, review, fix). Verify whether triage requires this variable and align the two guides.
There was a problem hiding this comment.
This guide is correct — verified against the harness definitions. Triage's runner_env does NOT include REPO_FULL_NAME; it only has GITHUB_ISSUE_URL and GH_TOKEN. The repo is derived from the issue URL. local-dev.md should be updated to match.
fullsend-ai-review
Bot
added
ready-for-merge
- Split "Core variables" into dedicated GCP credentials and GitHub tokens sections with context on CI vs local differences - Add GOOGLE_CLOUD_PROJECT (now required by gcp-vertex.env) - Explain that a single PAT with repo scope covers GH_TOKEN, PUSH_TOKEN, and REVIEW_TOKEN locally - Add gcloud command to create service account key - Note WIF/OIDC is CI-only; SA keys need no refresh locally - Move PUSH_TOKEN and REVIEW_TOKEN out of per-agent table into the GitHub tokens section where they belong Signed-off-by: Wayne Sun <gsun@redhat.com>
- Add GCP project prerequisite: Agent Platform API + Claude models in Model Garden - Expand GCP credentials section with full SA creation commands (create account, grant roles/aiplatform.user, create key) - Link to official GCP docs: Vertex AI Claude models, SA key creation, WIF - Expand GitHub tokens section with classic PAT creation link and explain why fine-grained PATs are not supported - Link to official GitHub docs for both PAT types Signed-off-by: Wayne Sun <gsun@redhat.com>
| # Create and download the key file | ||
| gcloud iam service-accounts keys create sa-key.json \ | ||
| --iam-account=fullsend-local@<project-id>.iam.gserviceaccount.com | ||
| chmod 600 sa-key.json |
There was a problem hiding this comment.
[low] correctness
The per-agent variables table lists REPO_FULL_NAME for "code, review, fix" agents, but docs/guides/dev/local-dev.md lists it for "triage, code, review, fix". One of the two guides has the wrong agent list for this variable.
Suggested fix: Verify which agents require REPO_FULL_NAME by checking harness definitions, then align both guides.
There was a problem hiding this comment.
Same as above — this guide's listing (code, review, fix) is correct per the harness definitions. Triage does not require REPO_FULL_NAME.
fullsend-ai-review
Bot
added
ready-for-merge
Signed-off-by: Wayne Sun <gsun@redhat.com>
Address review feedback: - Move install path from /usr/local/bin to $HOME/.local/bin - Restructure agent examples to show required env vars per agent instead of repeating identical commands Signed-off-by: Wayne Sun <gsun@redhat.com>
Signed-off-by: Wayne Sun <gsun@redhat.com>
- Add section on running customized agents locally with manual overlay of org-level and per-repo customized/ directories - Add prioritize and retro agent examples with env vars - Update per-agent variables table for all 6 built-in agents Signed-off-by: Wayne Sun <gsun@redhat.com>
| --role="roles/aiplatform.user" | ||
|
|
||
| # Create and download the key file | ||
| gcloud iam service-accounts keys create sa-key.json \ |
There was a problem hiding this comment.
[low] correctness
The per-agent variables table lists REPO_FULL_NAME for "code, review, fix, retro" and GITHUB_ISSUE_URL for "triage, prioritize", while docs/guides/dev/local-dev.md lists REPO_FULL_NAME for "triage, code, review, fix" and GITHUB_ISSUE_URL for only "triage". The two guides now diverge on both variables.
Suggested fix: Align both tables by verifying against harness definitions which agents actually require REPO_FULL_NAME, then update local-dev.md to match.
There was a problem hiding this comment.
This guide is correct — verified against the harness definitions on main:
REPO_FULL_NAME: code, review, fix, retro (all four have it inrunner_env)GITHUB_ISSUE_URL: triage, prioritize (both have it inrunner_env)- Triage does NOT have
REPO_FULL_NAME— it derives the repo from the issue URL
local-dev.md is the one that's stale and should be updated separately.
fullsend-ai-review
Bot
added
ready-for-merge
4 participants
Summary
docs/guides/user/running-agents-locally.md) covering the full workflow for running fullsend agents on macOS and Linux using released binaries — no Go toolchain or source checkout needed.fullsendconfig directory, creating env files with per-agent variables, running each agent type (triage, review, code, fix), cross-platform binary resolution, platform notes, and troubleshootingdocs/guides/README.mdunder User guidesSplit out from #662 to keep the feature PR focused on the code change.
Test plan
local-dev.md, GitHub Releases URL)docs/guides/README.mdentry renders correctly