Skip to content

refactor(registry/coder-labs/modules/sourcegraph-amp)!: remove agentapi, tasks and start logic - #1160

Open
35C4n0r wants to merge 9 commits into
mainfrom
35C4n0r/migrate-sourcegraph-amp
Open

35C4n0r wants to merge 9 commits into
mainfrom
35C4n0r/migrate-sourcegraph-amp

Conversation

@35C4n0r

@35C4n0r 35C4n0r commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Closes REG-86

Refactor coder-labs/sourcegraph-amp to install and configure Amp only, matching the claude-code, codex, copilot, and cursor-cli (#1152) migrations. Changes beyond the AgentAPI removal are limited to what the current Amp docs require.

  • Replace the agentapi module with coder-utils; drop Tasks/web-app inputs (order, group, cli_app, web_app_display_name, cli_app_display_name, install_agentapi, agentapi_version, report_tasks, ai_prompt, mode), the start script, the task_app_id output, and the task-reporting coder MCP server (which also rendered CODER_AGENT_TOKEN into settings).
  • Install with the official installer (https://ampcode.com/install.sh), passing amp_version as AMP_VERSION. With install_amp = true it always runs; with false the module only checks amp is on PATH. install_via_npm is removed.
  • workdir is optional and created if missing.
  • base_amp_config overwrites ~/.config/amp/settings.json on every start (no stale default keys), then mcp is added into amp.mcpServers with existing servers winning, matching amp mcp add. If the file is JSONC or invalid JSON, mcp is skipped with a warning.
  • instruction_prompt still writes ~/.config/AGENTS.md. amp_api_key is exported as AMP_API_KEY only when set and never rendered into the script.
  • No variable validation blocks. Add scripts output; README reduced to examples and doc links; rewrite tests. Bump to 4.0.0.

Important

Not verified in a live Amp session (no Amp access token): that MCP servers written by the module start. amp mcp list with the real binary lists them, and amp mcp doctor shows everything connected.

Decision log
Topic Amp reality (current docs, CLI 0.0.1791914780-gdfd130) Decision
Install install.sh honors AMP_VERSION (empty = latest), installs to ~/.amp/bin, links ~/.local/bin/amp Official installer on every start; no installed-version check
npm npm package wraps the same native binaries Remove install_via_npm
Mode --mode flag only; old free|rush|smart values are gone Drop mode, document the flag
MCP duplicates amp mcp add exits 1 with "already exists" and keeps the stored server (verified) Existing wins
Settings Amp accepts JSONC; AMP_SETTINGS_FILE overrides the path Apply mcp only to strict JSON, honor the override
base_amp_config Existing input; old defaults are not in the settings reference Keep the name; overwrite the file, then apply mcp; no defaults
Managed settings Not present before Not added
AGENTS.md Both ~/.config/AGENTS.md and ~/.config/amp/AGENTS.md are always loaded Keep the existing ~/.config/AGENTS.md
Validations Not needed for correctness None; amp_version is base64-encoded into the script

Testing

  • terraform test (6 passed), bun test main.test.ts (12 passed, container-based), terraform fmt, prettier, shellcheck on the rendered script
  • Rendered install script with the real installer in codercom/enterprise-node:latest: install, workdir, AGENTS.md, user shared server kept with a warning while everything and a base_amp_config server were added, unrelated keys preserved, amp mcp list shows all three, rerun succeeds
  • Dogfood workspace from a test template: JSON base_amp_config overwrites settings.json and mcp is added with base servers winning; JSONC base_amp_config is written as is and mcp is skipped with a warning

Generated with Coder Agents.

…pi, tasks and start logic

- Replace the agentapi module with coder-utils; drop Tasks/web-app inputs, start script, and the task-reporting MCP server.
- Install via the official installer (AMP_VERSION pin, temp file + bash -n, retrying curl); drop npm install.
- Make workdir optional; merge amp_settings (was base_amp_config) and mcp into ~/.config/amp/settings.json, existing MCP servers win.
- Add managed_settings (/etc/ampcode/managed-settings.json); write instruction_prompt to ~/.config/amp/AGENTS.md.
- Drop mode (use --mode in the launcher); add scripts output, main.tftest.hcl, container tests; bump to 4.0.0.
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Module Scorecard Check

coder-labs/sourcegraph-amp: first scorecard, 62 / 100

No specific score is required to contribute, but modules with higher scores are more likely to be approved by the Coder team and widely used.

Full scorecard for this PR
Presentation & Onboarding Agent Integration Credential Hygiene Restricted-Environment Readiness Engineering Quality Overall
10 / 17 10 / 25 20 / 20 7 / 20 10 / 10 62 / 100
Drilldown

Presentation & Onboarding — 10 / 17

Criterion Max Score Notes
Configuration-mode examples 12 10 Four README examples cover basic install, standalone launcher app, settings/MCP/guidance, and script serialization. However, the install_amp = false (pre-installed binary) mode—documented only in the variable description in main.tf—has no dedicated README example, leaving a major operational mode under-documented.
Visual preview 5 0 No embedded image, GIF, or video in the README. The icon frontmatter field references an SVG logo, which does not count as a preview of the module in action.

Credential Hygiene — 20 / 20

Criterion Max Score Notes
Secrets marked sensitive 16 16 amp_api_key is declared sensitive = true in main.tf. All README examples reference var.amp_api_key (a Terraform variable), never an inline literal or placeholder key.
Non-hardcoded auth path 4 4 The README Authentication section explicitly documents two paths: "Set amp_api_key (exported as AMP_API_KEY) or run amp login in the workspace." The amp login interactive path avoids pasting a raw key into templates.

Restricted-Environment Readiness — 7 / 20

Criterion Max Score Notes
Mirrorable artifact source 5 0 The install URL https://ampcode.com/install.sh is hardcoded in scripts/install.sh.tftpl. No module input variable overrides this URL. amp_version pins a release but does not change the download source.
Bring-your-own binary 10 5 install_amp = false skips the installer and validates an existing amp on PATH (tested in main.test.ts). The variable description in main.tf documents this, but the README contains no example or section demonstrating the pre-installed-binary path, making it under-documented for end users.
Egress transparency 3 0 No dedicated README section enumerates external endpoints. The only external call (https://ampcode.com/install.sh) is visible only in the install script source, not in a network/offline/air-gapped section.
Runs without sudo 2 2 The install script operates entirely within $HOME (~/.amp/bin, ~/.local/bin, shell profiles). No sudo invocation appears anywhere in scripts/install.sh.tftpl.

Engineering Quality — 10 / 10

Criterion Max Score Notes
Input quality 6 6 All 11 variables have clear descriptions and sensible defaults. Complex inputs (base_amp_config, mcp, instruction_prompt) include links to Amp documentation. amp_api_key is marked sensitive. Types are appropriate (bool for install_amp, string for the rest).
Test coverage 4 4 main.tftest.hcl covers business logic (defaults, workdir trimming, conditional env-var creation, version passthrough, scripts output ordering). main.test.ts provides 11 end-to-end tests executing real scripts in containers, covering happy path, pre-installed binary, installer download/failure, workdir creation, config overwrite, MCP merge semantics, JSONC handling, API-key leakage, and pre/post scripts.

Agent Integration — 10 / 25

Criterion Max Score Notes
AI governance 10 0 No mention of Coder AI Gateway or Agent Firewall in the README or any module file. The module installs and configures Amp but does not document how Coder governs auth, routing, or policy enforcement for the agent.
Dashboard entry point 5 5 The "Standalone mode with a launcher app" example in the README documents a coder_app resource with slug, display_name, icon, open_in = "slim-window", and a launch command.
Session continuity 5 0 No documentation of session resumption, session IDs, or persistent session managers (tmux, screen, boo) in the README or module files.
Managed configuration 5 5 The "Settings, MCP servers, and guidance" example documents base_amp_config (overwrites ~/.config/amp/settings.json), mcp (adds to amp.mcpServers), and instruction_prompt (writes ~/.config/AGENTS.md), with links to Amp docs for each.

Overall — 62 / 100

Raw 57 / 92 → round(57 / 92 × 100) = 62

Tip

You can run this locally by telling your agent: "review this module against .github/scorecard/SCORECARD.md".


Scored against SCORECARD.md with solstice-1. Language-model scores are advisory.

@matifali

Copy link
Copy Markdown
Member

Maybe we can repurpose this module to use Coder workspaces as Amp runners?
https://ampcode.com/docs/cli/runners

35C4n0r commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

Ack, will check this out.

@linear-code

linear-code Bot commented Oct 13, 2026

Copy link
Copy Markdown

REG-86

…to agentapi removal and current docs

Keep base_amp_config (merged, no defaults) instead of renaming it, drop managed_settings, variable validations, and installed-version checks, warn per skipped duplicate MCP server, keep ~/.config/AGENTS.md, and trim the README.
…ings with base_amp_config

base_amp_config now overwrites settings.json on every start instead of merging keys; mcp is applied afterwards.
@35C4n0r
35C4n0r marked this pull request as ready for review October 14, 2026 12:13
@coder

coder Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Automated review requested by @matifali. Please sanity-check before acting on it.

Verdict: request changes. No blockers. It's still a draft, and it has the same upgrade-leftover problem as #1159 (worse here: it includes an agent token), plus an undocumented change to MCP merge precedence.

What's good

  • main.tf is valid and minimal. data.external (which read CODER_AGENT_TOKEN at plan time), the external provider, coder_workspace* data sources, mode validation, agentapi and task_app_id are all removed. amp_api_key is now sensitive and the env var is only created when set.
  • No leftovers of the task/agentapi identifiers in module code or README (only the test-util import path and a negative assertion).
  • The install script fails loudly now. v3 swallowed installer failures with return 0.
  • Tests: a new main.tftest.hcl (6 runs; v3 had none, so AGENTS.md's tftest requirement is now met) and a rewritten main.test.ts (12 container tests).
  • The README warning (L23) names the removed inputs, the --mode replacement and the dropped default keys and coder MCP server.

Findings

  1. should-fix: the v3 amp.mcpServers.coder entry survives upgrades. v3 (main.tf:189-201 and configure_amp_settings on main) wrote a coder server into ~/.config/amp/settings.json. It carries CODER_MCP_APP_STATUS_SLUG=amp, CODER_MCP_AI_AGENTAPI_URL=http://localhost:3284, and plaintext CODER_AGENT_TOKEN and CODER_AGENT_URL. In v4, an empty base_amp_config means the file is never touched. With mcp set, setup_mcp (install.sh.tftpl:170) keeps existing servers, so the stale entry stays forever.
    Fix: drop .["amp.mcpServers"].coder when it matches the old signature (command == "coder" and env.CODER_MCP_APP_STATUS_SLUG present), or at least document manual removal in README L23.
  2. should-fix (docs): MCP precedence is reversed and not documented. v3 merged base config servers → coder → mcp, so mcp won on duplicate names. In v4, base_amp_config is written first and existing servers win, so base wins. The test base-config-overwrites-then-mcp-is-applied (main.test.ts:288-316) asserts this. Add a sentence to the README (L23 or L98).
  3. nit: required_version jumped from >= 1.0 to >= 1.9 (main.tf:2). No feature here needs 1.9 (no validation blocks); opencode stays at >= 1.0. Revert, or call it out as breaking in the migration note.
  4. nit: amp_version = "latest" is passed through literally (install.sh.tftpl:104). opencode maps latest→empty (opencode install.sh.tftpl:107-110) and the series default is latest elsewhere. Map it for parity, or document that only empty means latest.
  5. nit: test gaps. Nothing tests the AMP_SETTINGS_FILE override (install.sh.tftpl:18), even though the decision log advertises it (opencode has the equivalent config-dir-env-is-respected). The tftest has no assertions that instruction_prompt, base_amp_config or mcp are encoded into the script, and nothing tests that an empty amp_version gives an empty AMP_VERSION.
  6. nit: ln -s on a dangling symlink (install.sh.tftpl:76-77). Same issue as opencode; use ln -sfn.
  7. nit: tests import from agentapi/test-util (main.test.ts:18-21). Same series-level concern as opencode.
  8. nit: confirm the amp --mode medium example (README L52). The old free|rush|smart values are gone. I couldn't confirm "medium" from public docs (the SDK docs show low). Verify it against amp --help.
  9. nit: missing note about leftover state. The old ~/.amp-module/ dir and v3-written default keys (amp.anthropic.thinking.enabled, amp.todos.enabled, amp.terminal.animation) stay in existing settings files. That's harmless, but worth a line in the migration note.

CI and reviews

  • Checks on c722dd6: README validation ✅, typos/format ✅, lint ✅, "Validate Terraform output" ✅ (TS tests, terraform tests, validate, shellcheck). Score comparison still in progress.
  • No reviews or review comments. matifali suggested repurposing the module for Amp runners (ampcode.com/docs/cli/runners). The author acknowledged it, and it's not addressed in this PR. Fine as a follow-up, but it should get a Linear issue.
  • The PR is still a draft, and the author notes that MCP servers starting in a live Amp session was not verified (no access token).

Consistency with the series

  • It matches opencode, codex and claude-code: same script layout, helper functions, curl flags, JSONC skip and scripts output.
  • Two differences from opencode: the required_version bump (nit 3) and latest handling (nit 4). There's also no installer-run retry (opencode retries 3x), which is fine unless Amp's installer has similar flakiness.

…rsion >= 1.0

Nothing in the module needs Terraform 1.9.
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.

2 participants