feat: add Amp agent plugin#1774
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ed0c2f4c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds a new built-in Amp agent plugin and wires it into the core/CLI/web plugin registries so AO can detect and launch Amp sessions, including activity classification and workspace hook setup.
Changes:
- Introduces
@aoagents/ao-plugin-agent-ampwith activity detection, liveness checks, restore command support, and wrapper hook setup. - Registers the Amp agent plugin as a built-in across core (
plugin-registry), CLI (direct import + detection), and web services (static import + registry). - Extends the core
Agentinterface with an optionalpromptDeliveryflag intended to support post-launch prompt delivery.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds the new workspace package and its dependencies (which, @types/which). |
| packages/web/src/lib/services.ts | Registers the Amp agent plugin in the Next.js service registry via static import. |
| packages/web/package.json | Adds @aoagents/ao-plugin-agent-amp dependency for web bundling. |
| packages/plugins/agent-amp/tsconfig.json | TypeScript build configuration for the new plugin package. |
| packages/plugins/agent-amp/src/index.ts | Implements the Amp agent plugin (launch, env, activity, liveness, restore, hooks). |
| packages/plugins/agent-amp/src/index.test.ts | Adds unit tests covering manifest/export shape, detection, activity/liveness, and hooks. |
| packages/plugins/agent-amp/package.json | Defines the new plugin package metadata, scripts, and dependencies. |
| packages/core/src/types.ts | Adds Agent.promptDelivery?: "inline" | "post-launch". |
| packages/core/src/plugin-registry.ts | Adds Amp to the built-in plugin list. |
| packages/cli/src/lib/plugins.ts | Adds a direct import/registration mapping for the Amp agent plugin. |
| packages/cli/src/lib/detect-agent.ts | Adds Amp to the agent auto-detection list. |
| packages/cli/package.json | Adds @aoagents/ao-plugin-agent-amp dependency for CLI builds. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Greptile SummaryThis PR adds a full Amp CLI agent plugin (
Confidence Score: 5/5Safe to merge. All critical cross-platform guards are in place and the only finding is a minor style inconsistency in an embedded subprocess script. The implementation is a clean port of the Forge/opencode plugin blueprint. packages/plugins/agent-amp/src/index.ts — the AMP_PROMPT_LAUNCHER_SCRIPT constant and its buildAmpLaunchCommand call site.
|
| Filename | Overview |
|---|---|
| packages/plugins/agent-amp/src/index.ts | Core Amp agent plugin implementing all required Agent interface methods. Follows the Forge/opencode plugin pattern closely with one minor cross-platform guide violation in the embedded launcher script. |
| packages/plugins/agent-amp/src/index.test.ts | Comprehensive test suite covering all plugin boundaries including the Windows platform mock for the tmux liveness guard. |
| packages/cli/src/lib/plugins.ts | Adds ampPlugin to the CLI's static plugin registry. |
| packages/core/src/plugin-registry.ts | Registers Amp in the core built-in plugin list. |
| packages/web/src/lib/services.ts | Registers Amp plugin in the web service plugin registry. |
| packages/plugins/agent-amp/package.json | Correct package scaffold with workspace dependency on ao-core, which for binary detection, and standard build/test/typecheck scripts. |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/plugins/agent-amp/src/index.ts:62-73
The embedded launcher script uses `process.platform==='win32'` directly, which violates the cross-platform guide's rule to always use `isWindows()` from `@aoagents/ao-core`. Since this runs inside a subprocess string, `isWindows()` can't be imported there — but the value can be computed in TypeScript and passed through the existing JSON payload, making it testable and consistent with the rest of the codebase.
```suggestion
const AMP_PROMPT_LAUNCHER_SCRIPT = [
'const {spawn}=require("node:child_process");',
'const fs=require("node:fs");',
"const payload=JSON.parse(process.argv[1]);",
"const promptParts=Array.isArray(payload.promptParts)?payload.promptParts:[];",
"const input=promptParts.map((part)=>part.type==='file'?fs.readFileSync(part.value,'utf8'):String(part.value??'')).filter(Boolean).join('\\n\\n');",
"const args=Array.isArray(payload.args)?payload.args:[];",
"const child=spawn('amp',args,{stdio:['pipe','inherit','inherit'],shell:payload.shell===true,windowsHide:true});",
"child.on('error',(err)=>{console.error(err?.message||String(err));process.exit(1);});",
"child.on('exit',(code)=>process.exit(code??0));",
"child.stdin.end(input);",
].join("");
```
### Issue 2 of 2
packages/plugins/agent-amp/src/index.ts:100-101
To complete the fix, pass `isWindows()` into the payload so the subprocess script reads it from there rather than checking `process.platform` itself.
```suggestion
const payload = JSON.stringify({ args, promptParts, shell: isWindows() });
return `node -e ${shellEscape(AMP_PROMPT_LAUNCHER_SCRIPT)} ${shellEscape(payload)}`;
```
Reviews (3): Last reviewed commit: "fix: avoid shell pipe for amp prompts" | Re-trigger Greptile
Summary
Validation