feat(watch): post-cutover monitoring that flags broken redirects and hands back the fix - #33
Merged
Merged
Conversation
Exporting a redirect file is a prediction. It is deployed by a human into a stack Redirx never sees, and every common failure is invisible from our side at export time: the rules never shipped, a CDN rule shadowed them, a trailing-slash normaliser turned one hop into three, someone shipped 302. A watch closes that loop by asking the live site what it actually does with each approved old URL, and ranks what it finds by the traffic at stake — the 026 baseline exists for exactly this. ## Probing Redirects are followed one hop at a time rather than via allow_redirects, because the diagnosis lives in the shape of the chain: how many hops, whether they were permanent, and where it actually landed. Every hop is re-validated against the SSRF rules — following a redirect means letting another server choose our next URL, and `Location: http://169.254.169.254/` is a thing a server is allowed to say. HEAD first, retried as GET on the statuses that mean "this server dislikes HEAD" rather than "this URL is broken". Sweeps get their own rate-limiter namespace, paced slower than discovery. Unlike a crawl this recurs forever: a rate a site tolerates once is one it will be asked to tolerate every few hours for months. ## Two normalisations, deliberately different Target comparison ignores scheme and `www.`, so an HSTS hop is not reported as a wrong target on every URL of a site. Loop detection cannot use that: measured against github.com and google.com, reusing it reported both as redirect loops, which would have been the verdict on most of the web. `visit_identity` answers "did we already request this exact URL", and a `/a` -> `/a/` -> `/a` oscillation is still caught. Both directions are regression-tested. ## Reporting Issues are current state keyed by URL, not an event log. A redirect broken for a week is one row and one email, not seven of each; `alerted_at` closes it, and a *different* failure at the same URL reopens it because that is news. Transient types wait for a second consecutive sighting before waking anyone — one failed request is a blip. Closing an issue requires having actually re-probed the URL. Absence from a sweep's findings is not evidence of repair: the per-sweep cap leaves a large site's tail unchecked, and treating that as fixed would silently close real breakage on exactly the sites that most need monitoring. ## Where it runs The worker, not the API. Probing a few thousand URLs takes tens of minutes and the API is a single sync gunicorn worker where that would block every other request. One sweep at a time, alongside jobs rather than taking a job slot, since a sweep is almost entirely idle time waiting on the rate limiter. A long lease instead of a lease-extension loop: nothing is waiting on the result, so a stale lease costs monitoring latency, not a user-visible delay. ## Tests 90 new. The classifier's are mostly cases that must *not* be flagged — a monitor that cries wolf gets muted, and then it reports nothing. Hop following, HEAD fallback and loop termination are tested against a real local aiohttp server, with the SSRF guard asserted separately since it is what makes that server unreachable in production. Backend suite 421 tests; pre-existing failure count unchanged (3 failures, 15 errors, all from the known local SUPABASE_KEY mismatch). Migration 029 applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The monitoring half says what broke. This closes the loop: turn each flagged
issue into a redirect Redirx would deploy, and hand it back in a file the
customer can ship through the process they already have.
## The fix is never a new guess
Every proposed target resolves to something already justified — the mapping the
user approved, or the destination the site itself proved reachable:
missing / 404 / wrong target -> the approved mapping
extra hops -> where the chain already ends, collapsed to one
302 -> same target, issued as a 301
loop / outage / 5xx -> nothing
That last row matters. A loop is not repaired by choosing a better destination,
and emitting a blank target would replace a broken rule with one that breaks
differently. Those rows are omitted from the patch rather than shipped empty.
Notably, `wrong_target` proposes the *approved* mapping, not the URL the probe
observed. Proposing where it actually landed would ratify the bug — that
destination is the symptom.
## Same formatters, on purpose
`fix_rows()` emits old_url/new_url so `redirect_export` renders the correction
in all eight formats unchanged. A fix the customer cannot deploy the way they
deployed the original is not a fix. `paths` stays the default for the same
reason it is elsewhere: an absolute source loads fine and silently redirects
nothing.
## Agent surface
`POST/GET /api/v1/migrations/{id}/watch` and `GET .../watch/fixes`. The
zero-touch claim did not end at export — an agent that deploys a redirect file
had no way to learn whether the deploy worked, and only the live site can
answer that. `checked` is returned separately from the issue count so an agent
polling right after starting a watch can tell "clean" from "not looked yet".
## UI
A watch page leading with clicks at risk, and a prompt on the review page that
links to the existing watch rather than offering a second one — a returning
user must not double the probe traffic aimed at their own origin just by
revisiting.
Backend 436 tests, frontend 285; pre-existing backend failures unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Checked against production: `old_site_domain` is NULL on all 143 sessions — the column exists but nothing writes it. A watch that relied on it would never start, so deriving the host from the mappings is the normal path and the comment saying otherwise was misleading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 20, 2026
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
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.
Closes claims #2 (post-cutover traffic and 404 monitoring with email alerts) and #3 (auto flag and fix).
Exporting a redirect file is a prediction. It gets deployed by a human into a stack Redirx never sees, and every common failure is invisible from our side at export time: the rules never shipped, a CDN rule shadowed them, a trailing-slash normaliser turned one hop into three, someone shipped 302. A watch asks the live site what it actually does with each approved old URL, ranks the failures by the traffic they cost, and hands back a corrective file.
The 026 traffic baseline was built for this — its own migration note says "monitoring and recovery reporting depend on it existing."
A real bug the unit tests could not see
Probing real sites during development caught this, and it would have made the feature useless:
http://github.comandhttp://google.comwere both reported as redirect loops.Loop detection reused
normalize_for_compare, which deliberately ignores scheme andwww.so an HSTS hop is not flagged as a wrong target on every URL of a site. Turned on loop detection, that same rule sayshttp://xandhttps://xare the same URL — so every site doing http→https or bare→www, i.e. most of the web, came back broken.The fix separates the two questions.
normalize_for_compareanswers "same page?";visit_identityanswers "same request?" and keeps scheme, host and trailing slash. A genuine/a→/a/→/aoscillation is still caught. Both directions are regression-tested, at the unit level and against a live server.Probing
allow_redirects— the diagnosis lives in the shape of the chain: hop count, permanent vs temporary, and where it actually landed.Location: http://169.254.169.254/is a thing a server is allowed to say.Reporting, without becoming noise
Issues are current state keyed by URL, not an event log. A redirect broken for a week is one row and one email, not seven of each.
alerted_atgates re-reporting; a different failure at the same URL reopens it, because that is news. Transient types wait for a second consecutive sighting — one failed request is a blip.Closing an issue requires having re-probed that URL. Absence from a sweep's findings is not evidence of repair: the per-sweep cap leaves a large site's tail unchecked, and treating that as fixed would silently close real breakage on exactly the sites that most need monitoring.
The fix half
Every proposed target resolves to something already justified — the approved mapping, or the destination the site itself proved reachable:
wrong_targetproposes the approved mapping, not the URL the probe observed — proposing where it actually landed would ratify the bug. Rows with no actionable target are omitted rather than emitted blank, since a half-filled rule replaces a broken redirect with a differently-broken one.fix_rows()shapes issues as old_url/new_url soredirect_exportrenders the correction in all eight existing formats. A fix the customer cannot deploy the way they deployed the original is not a fix.Where it runs
The worker, not the API. Probing a few thousand URLs takes tens of minutes and the API is a single sync gunicorn worker where that would block every other request. One sweep at a time, running alongside jobs rather than taking a job slot, since a sweep is almost entirely idle time waiting on the rate limiter. A long lease instead of a lease-extension loop: nothing is waiting on the result, so a stale lease costs monitoring latency rather than a user-visible delay.
Agent surface
POST/GET /api/v1/migrations/{id}/watchandGET .../watch/fixes. The zero-touch story from #31 did not actually end at export — an agent that deploys a redirect file had no way to learn whether the deploy worked, and only the live site can answer that.checkedis returned separately from the issue count so an agent polling right after starting a watch can distinguish "clean" from "not looked yet".Tests
96 new backend, 5 frontend. The classifier's are mostly cases that must not be flagged — a monitor that cries wolf gets muted, and then it reports nothing. Hop following, HEAD fallback, canonical hops and loop termination are tested against a real local aiohttp server, with the SSRF guard asserted separately since it is what makes that server unreachable in production.
Backend 436 tests, frontend 285. Pre-existing backend failure count unchanged (3 failures, 15 errors — all the known local
SUPABASE_KEYmismatch).Migration 029 applied.
Not in this PR
🤖 Generated with Claude Code