Skip to content

Commit 8d9bdd6

Browse files
fix(amp): replace the impossible phx-watch-pr install command (#137)
README and docs/amp.md told users to install the watcher with `amp plugins add` against a raw.githubusercontent.com URL. Amp rejects that outright: Error: For now, only https://ampcode.com/@amp/plugins/*.ts and https://ampcode.com/@amp/plugins/*.ts URLs are allowed. Verified on Amp 0.0.1787045288. Nothing is written, so the documented path to the watcher never worked for anyone. docs/amp.md already contradicted itself about this: 390 lines below the install block it states that Amp "restricts `amp plugins add` ... to Amp-hosted plugins" and hands out an atomic curl for elixir-phoenix.ts. The install instruction was simply the wrong half of the contradiction. Both call sites now use that same curl + mv pattern against the standalone repository's stable branch, verified to fetch bytes identical to the canonical target. The update section loops over both plugins rather than only the workflow one. Also raises the phx-watch-pr harness subprocess timeout from 60s to 240s. The harness drives a fake clock with real 80ms flushes -- advance() runs up to 21 per call across 23 calls -- so it spends ~59s waiting for ~3s of CPU. Consecutive local runs took 58.9s, 60.4s, and 73.9s, so a required check was passing or failing on runner load alone. The harness is unchanged; shortening the flush would swap a timeout flake for a race. Fixes a paragraph-join in README introduced when the watch-pr block was merged forward in #132.
1 parent 7c613bd commit 8d9bdd6

4 files changed

Lines changed: 52 additions & 17 deletions

File tree

CHANGELOG.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,27 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
9999

100100
### Fixed
101101

102+
- **The `phx-watch-pr` harness test was one second from red** — it ran with a
103+
60-second subprocess timeout and takes ~59 seconds of wall time for ~3 seconds
104+
of CPU: the harness drives a fake clock with real 80ms flushes, and `advance()`
105+
performs up to 21 of them across 23 calls. Observed locally at 58.9s, 60.4s,
106+
and 73.9s on consecutive runs, so it passes or fails on runner load alone —
107+
and it is a required check, meaning it would have started blocking unrelated
108+
PRs. Raised to 240s; the harness itself is unchanged, since shortening the
109+
flush would trade a timeout flake for a race.
110+
111+
- **The `phx-watch-pr` install command could never have worked** — README and
112+
`docs/amp.md` told users to run `amp plugins add` against a
113+
`raw.githubusercontent.com` URL. Amp `0.0.1787045288` rejects that with
114+
`For now, only https://ampcode.com/@amp/plugins/*.ts ... URLs are allowed` and
115+
writes nothing. The same `docs/amp.md` already said, 390 lines further down,
116+
that "Amp currently restricts `amp plugins add` ... to Amp-hosted plugins" —
117+
the file contradicted itself, and the install instruction was the wrong half.
118+
Both call sites now use the atomic `curl` + `mv` pattern already documented
119+
for `elixir-phoenix.ts`, pointing at the standalone repository's `stable`
120+
branch. The update section covers both plugins instead of only the workflow
121+
one.
122+
102123
- **Upgrading from v2.x no longer lands users in a dead install** (reported by
103124
@barquesurlocean, #135) — v3.0.0 renamed the plugin manifest to `phx` (which
104125
is what makes `/phx:*` correct; pre-v3 versions namespaced their commands as

README.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,13 +306,17 @@ hosted artifacts are managed through Amp's Personal Settings or personal Git
306306
repositories and work across machines and orbs. See the complete [Amp guide](docs/amp.md)
307307
for hosted publication, source precedence, wrapper limits, updates, fallback
308308
installation, specialists, safety boundaries, and verification.
309+
309310
`phx-watch-pr` additionally needs the separate `phx-watch-pr.ts` plugin.
310311
Install it into the project where the worker Orb opens and watches PRs:
311312

312313
```bash
313-
amp plugins add \
314-
https://raw.githubusercontent.com/oliver-kriska/claude-elixir-phoenix/main/targets/amp/plugins/phx-watch-pr.ts \
315-
--target workspace
314+
mkdir -p .amp/plugins
315+
plugin=".amp/plugins/phx-watch-pr.ts"
316+
temporary="$(mktemp "${plugin}.XXXXXX")"
317+
curl --fail --silent --show-error --location \
318+
https://raw.githubusercontent.com/oliver-kriska/amp-elixir-phoenix/stable/plugins/phx-watch-pr.ts \
319+
--output "$temporary" && mv "$temporary" "$plugin"
316320
```
317321

318322
It holds a bounded Orb keep-alive lease, filters deployment-like checks out of

docs/amp.md

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,12 @@ generated Amp plugin because only the Plugin API can hold an Orb keep-alive
112112
lease and wake the same thread after inactivity:
113113

114114
```bash
115-
amp plugins add \
116-
https://raw.githubusercontent.com/oliver-kriska/claude-elixir-phoenix/main/targets/amp/plugins/phx-watch-pr.ts \
117-
--target workspace
115+
mkdir -p .amp/plugins
116+
plugin=".amp/plugins/phx-watch-pr.ts"
117+
temporary="$(mktemp "${plugin}.XXXXXX")"
118+
curl --fail --silent --show-error --location \
119+
https://raw.githubusercontent.com/oliver-kriska/amp-elixir-phoenix/stable/plugins/phx-watch-pr.ts \
120+
--output "$temporary" && mv "$temporary" "$plugin"
118121
```
119122

120123
Start a fresh Amp process or run `plugins: reload`. Plugins execute code; audit
@@ -501,18 +504,22 @@ normal update path, but removal followed by installation is the exact-sync path
501504
when a release deletes or renames a skill.
502505

503506
Amp currently restricts `amp plugins add` and directive-based auto-updates to
504-
Amp-hosted plugins. Update this GitHub-hosted plugin by downloading the current
505-
validated file again, or remove it with Amp:
507+
Amp-hosted plugins; it rejects any other URL outright. Update these
508+
GitHub-hosted plugins by downloading the current validated file again, or remove
509+
them with Amp:
506510

507511
```bash
508-
# Update the workspace plugin atomically
509-
plugin=".amp/plugins/elixir-phoenix.ts"
510-
temporary="$(mktemp "${plugin}.XXXXXX")"
511-
curl --fail --silent --show-error --location \
512-
https://raw.githubusercontent.com/oliver-kriska/amp-elixir-phoenix/stable/plugins/elixir-phoenix.ts \
513-
--output "$temporary" && mv "$temporary" "$plugin"
514-
515-
# Remove the workspace-scoped installation
512+
# Update a workspace plugin atomically
513+
for name in elixir-phoenix phx-watch-pr; do
514+
plugin=".amp/plugins/${name}.ts"
515+
[ -f "$plugin" ] || continue
516+
temporary="$(mktemp "${plugin}.XXXXXX")"
517+
curl --fail --silent --show-error --location \
518+
"https://raw.githubusercontent.com/oliver-kriska/amp-elixir-phoenix/stable/plugins/${name}.ts" \
519+
--output "$temporary" && mv "$temporary" "$plugin"
520+
done
521+
522+
# Remove a workspace-scoped installation
516523
amp plugins remove elixir-phoenix.ts --target workspace
517524
```
518525

scripts/tests/test_amp.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,10 @@ def test_native_watch_plugin_lifecycle_harness(tmp_path) -> None:
443443
cwd=Path(__file__).parents[2],
444444
text=True,
445445
capture_output=True,
446-
timeout=60,
446+
# The harness drives a fake clock with real 80ms flushes: advance() runs
447+
# up to 21 of them and is called 23 times, so the run is ~59s of waiting
448+
# against ~3s of CPU. A 60s limit left under a second of margin.
449+
timeout=240,
447450
)
448451

449452
assert result.returncode == 0, result.stderr or result.stdout

0 commit comments

Comments
 (0)