feat(agentapi): add agentapi_cache_dir for persistent binary caching#769
feat(agentapi): add agentapi_cache_dir for persistent binary caching#769iamriajul wants to merge 2 commits intocoder:mainfrom
Conversation
Add an opt-in agentapi_cache_dir variable. When set, the AgentAPI binary is stored in that directory after the first download and reused on subsequent workspace starts — useful with a shared persistent volume to avoid repeated GitHub downloads. The binary is cached under a versioned filename (e.g. agentapi-linux-amd64-v0.10.0) so multiple versions can coexist. When agentapi_version is "latest", the actual release tag is resolved via GitHub's redirect before computing the cache key, preventing the cache from getting stuck on a stale binary. The resolution request is skipped entirely when agentapi_cache_dir is not set. Bumps version 2.1.1 → 2.2.0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an opt-in persistent caching mechanism for the AgentAPI binary in the agentapi Terraform module to avoid repeated downloads on workspace start, with tests and documentation updates.
Changes:
- Introduces
agentapi_cache_dirTerraform variable and wires it into the install/start script. - Updates
main.shto reuse a cached versioned AgentAPI binary (and resolvelatestto a concrete tag for cache keying). - Adds module tests and README documentation for the new caching behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
registry/coder/modules/agentapi/scripts/main.sh |
Adds cache lookup/save logic and latest version resolution for cache key stability. |
registry/coder/modules/agentapi/main.tf |
Adds agentapi_cache_dir input and passes it to the startup script. |
registry/coder/modules/agentapi/main.test.ts |
Adds tests validating cache-hit and cache-save behavior. |
registry/coder/modules/agentapi/README.md |
Bumps module version and documents binary caching usage. |
.gitignore |
Ignores .serena directory. |
| "https://github.com/coder/agentapi/releases/latest" | grep -oE 'v[0-9]+\.[0-9]+\.[0-9]+' || echo "latest") | ||
| echo "Resolved AgentAPI latest version to: ${resolved_version}" | ||
| fi | ||
| cached_binary="${CACHE_DIR}/${binary_name}-${resolved_version}" |
There was a problem hiding this comment.
When resolving latest, the fallback || echo "latest" means a transient network/redirect failure will cache under the literal key "latest" and can pin the cache to a stale binary (contradicting the intended "never stale" behavior). Consider disabling caching for that run when the tag cannot be resolved (e.g., leave cached_binary empty if resolved_version is still "latest"), or treat resolution failure as a warning and proceed without writing/using a cache entry.
| "https://github.com/coder/agentapi/releases/latest" | grep -oE 'v[0-9]+\.[0-9]+\.[0-9]+' || echo "latest") | |
| echo "Resolved AgentAPI latest version to: ${resolved_version}" | |
| fi | |
| cached_binary="${CACHE_DIR}/${binary_name}-${resolved_version}" | |
| "https://github.com/coder/agentapi/releases/latest" | grep -oE 'v[0-9]+\.[0-9]+\.[0-9]+' || true) | |
| if [ -z "${resolved_version}" ]; then | |
| echo "Warning: Failed to resolve latest AgentAPI version tag; proceeding without cache for this run." | |
| else | |
| echo "Resolved AgentAPI latest version to: ${resolved_version}" | |
| fi | |
| fi | |
| if [ -n "${resolved_version}" ] && [ "${resolved_version}" != "latest" ]; then | |
| cached_binary="${CACHE_DIR}/${binary_name}-${resolved_version}" | |
| else | |
| cached_binary="" | |
| fi |
| "https://github.com/coder/agentapi/releases/latest" | grep -oE 'v[0-9]+\.[0-9]+\.[0-9]+' || echo "latest") | ||
| echo "Resolved AgentAPI latest version to: ${resolved_version}" | ||
| fi | ||
| cached_binary="${CACHE_DIR}/${binary_name}-${resolved_version}" |
There was a problem hiding this comment.
resolved_version is derived from AGENTAPI_VERSION and then interpolated into a filesystem path. Because agentapi_version can be an arbitrary string, this can introduce path traversal or invalid filenames (e.g., containing /, .., newlines) and cause writes outside CACHE_DIR when caching. Please sanitize the version component before building cached_binary (or restrict agentapi_version to a safe pattern when agentapi_cache_dir is set).
| cached_binary="${CACHE_DIR}/${binary_name}-${resolved_version}" | |
| # Sanitize the resolved version so it is safe to use as a filename component. | |
| # Allow only alphanumerics, dots, underscores, and hyphens; replace others with '_'. | |
| safe_resolved_version=$(printf '%s' "${resolved_version}" | tr -c 'A-Za-z0-9._-' '_') | |
| if [ -z "${safe_resolved_version}" ]; then | |
| echo "Warning: Resolved AgentAPI version '${resolved_version}' is empty after sanitization; skipping cache." | |
| cached_binary="" | |
| else | |
| cached_binary="${CACHE_DIR}/${binary_name}-${safe_resolved_version}" | |
| fi |
| mkdir -p "${CACHE_DIR}" | ||
| cp agentapi "${cached_binary}" |
There was a problem hiding this comment.
Caching is an optimization, but with set -e a failure in mkdir -p "${CACHE_DIR}" or cp agentapi "${cached_binary}" will abort the whole install/start even though the binary was successfully downloaded. Make cache writes best-effort (emit a warning and continue) so a misconfigured or read-only cache dir doesn’t break workspace startup.
| mkdir -p "${CACHE_DIR}" | |
| cp agentapi "${cached_binary}" | |
| if ! mkdir -p "${CACHE_DIR}"; then | |
| echo "Warning: Failed to create cache directory ${CACHE_DIR}. Continuing without caching." | |
| elif ! cp agentapi "${cached_binary}"; then | |
| echo "Warning: Failed to cache AgentAPI binary to ${cached_binary}. Continuing without caching." | |
| fi |
| if [ -n "${cached_binary}" ]; then | ||
| echo "Caching AgentAPI binary to ${cached_binary}" | ||
| mkdir -p "${CACHE_DIR}" | ||
| cp agentapi "${cached_binary}" |
There was a problem hiding this comment.
Writing the cache file via cp agentapi "${cached_binary}" is not atomic. If multiple workspaces share the same persistent cache volume, concurrent starts can race and one workspace may read a partially-written binary. Consider writing to a temp file in CACHE_DIR and then mv-ing into place (atomic rename), optionally with a lock to serialize writers.
| cp agentapi "${cached_binary}" | |
| tmp_cached_binary="${cached_binary}.$$" | |
| cp agentapi "${tmp_cached_binary}" | |
| mv -f "${tmp_cached_binary}" "${cached_binary}" |
| ARG_AGENTAPI_CHAT_BASE_PATH='${local.agentapi_chat_base_path}' \ | ||
| ARG_TASK_ID='${try(data.coder_task.me.id, "")}' \ | ||
| ARG_TASK_LOG_SNAPSHOT='${var.task_log_snapshot}' \ | ||
| ARG_CACHE_DIR='${var.agentapi_cache_dir}' \ |
There was a problem hiding this comment.
agentapi_cache_dir is interpolated directly into a single-quoted shell assignment. If the path contains a single quote or newline, it can break the generated script. Consider passing this value the same way as ARG_WORKDIR (base64-encoded/decoded) or otherwise escaping it before embedding it into the heredoc.
| ARG_CACHE_DIR='${var.agentapi_cache_dir}' \ | |
| ARG_CACHE_DIR="$(echo -n '${base64encode(var.agentapi_cache_dir)}' | base64 -d)" \ |
| const cachedBinary = `${cacheDir}/agentapi-linux-amd64-${pinnedVersion}`; | ||
| await execContainer(id, [ | ||
| "bash", | ||
| "-c", | ||
| `mkdir -p ${cacheDir} && cp /usr/bin/agentapi ${cachedBinary}`, | ||
| ]); |
There was a problem hiding this comment.
This test hardcodes the cached binary name as agentapi-linux-amd64-..., but the install script selects agentapi-linux-arm64 on aarch64. On ARM hosts (e.g., Apple Silicon / Colima), this will miss the cache and exercise the download path unexpectedly. Consider deriving binary_name by querying uname -m inside the container and building the expected cache filename accordingly.
| }, | ||
| }); | ||
|
|
||
| const cachedBinary = `${cacheDir}/agentapi-linux-amd64-${pinnedVersion}`; |
There was a problem hiding this comment.
This test assumes the cached filename uses agentapi-linux-amd64-..., which will fail on ARM (aarch64) where the script caches agentapi-linux-arm64-.... Consider computing the expected filename from the container architecture so the test is portable across dev environments.
| const cachedBinary = `${cacheDir}/agentapi-linux-amd64-${pinnedVersion}`; | |
| // Determine the container architecture so the expected cached filename | |
| // matches how the script names the AgentAPI binary. | |
| const archResult = await execContainer(id, ["uname", "-m"]); | |
| expect(archResult.exitCode).toBe(0); | |
| const arch = archResult.stdout.trim(); | |
| const agentArch = | |
| arch === "aarch64" || arch === "arm64" ? "linux-arm64" : "linux-amd64"; | |
| const cachedBinary = `${cacheDir}/agentapi-${agentArch}-${pinnedVersion}`; |
- Pass agentapi_cache_dir base64-encoded to avoid single-quote injection in the heredoc (same pattern as ARG_WORKDIR) - Skip cache entirely if latest version resolution fails instead of falling back to the literal string "latest" as a cache key - Sanitize resolved_version before using it as a filename component to prevent path traversal (tr -c 'A-Za-z0-9._-' '_') - Make cache writes best-effort: warn and continue if mkdir or cp fails so a misconfigured/read-only cache dir doesn't abort workspace startup - Write cache atomically via cp + mv to avoid concurrent workspaces on a shared volume observing a partially-written binary - Derive container architecture in tests via uname -m so cache filename is correct on both amd64 and arm64 hosts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Description
Add an opt-in
agentapi_cache_dirvariable to avoid re-downloading the AgentAPI binary on every workspace start. When set, the binary is stored in that directory after the first download and reused on subsequent starts — useful with a shared persistent volume.The binary is cached with a versioned filename (e.g.
agentapi-linux-amd64-v0.10.0) so multiple versions coexist safely. Whenagentapi_version = "latest", the actual release tag is resolved via GitHub's redirect before computing the cache key, so the cache never gets stuck on a stale binary. The resolution request is skipped entirely whenagentapi_cache_diris not set.Type of Change
Module Information
Path:
registry/coder/modules/agentapiNew version:
v2.2.0Breaking change: [ ] Yes [x] No
Testing & Validation
bun test— all agentapi-specific tests pass)bun fmt)Related Issues
None