fix(amp): replace the impossible phx-watch-pr install command - #137
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Post-merge fixes for #132, from an Amp review of the merged
mainplus oneissue found while verifying it.
1. The watcher install command could never have worked
README and
docs/amp.mdboth told users to run:Amp rejects this outright (verified on
0.0.1787045288):Nothing is written.
docs/amp.mdalready contradicted itself — 390 lines belowthe install block it states Amp "restricts
amp plugins add... to Amp-hostedplugins" and hands out an atomic
curlforelixir-phoenix.ts. The installinstruction was the wrong half of that contradiction.
Both call sites now use the same
curl+mvpattern against the standalonerepository's
stablebranch. Verified end to end: the documented commandfetches 43,318 bytes byte-identical to
targets/amp/plugins/phx-watch-pr.ts.The update section now loops over both plugins instead of only the workflow one.
2. The
phx-watch-prharness was one second from redIt ran with a 60s subprocess timeout and takes ~59s wall for ~3s CPU: the
harness drives a fake clock with real 80ms flushes, and
advance()performs upto 21 of them across 23 calls. Three consecutive local runs took 58.9s, 60.4s,
and 73.9s — a required check passing or failing on runner load alone, which
would have started blocking unrelated PRs.
Raised to 240s. The harness is unchanged; shortening the flush would trade a
timeout flake for a race.
3. Paragraph join
Fixes a missing blank line in README introduced when the watch-pr block was
merged forward in #132.
Verification
make ci— 246 passed, all five manifests validated, all four generatedtargets drift-clean, snapshots match, eval passed (51 skills avg 0.990, 26
agents 1.000).
Not in this PR
amp-elixir-phoenixstablestillserves the pre-compaction 98,905-byte
elixir-phoenix.ts(canonical is95,191, under the 96,000 transport budget) and three skills differ. Needs a
fresh sync from
7c613bd.Notify Docs Sitefails with Bad credentials (401) —SITE_DISPATCH_TOKENis invalid or expired and needs regenerating, not rescoping.