diff --git a/README.md b/README.md index 1bc5621..4fb36a6 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,9 @@ What does not work yet: - **Any notification.** Nothing is emailed, posted or pushed anywhere: there is no mail configuration and no secret for one. `GET /api/alerts` lists every domain whose last DNS check did not find its token, and every domain no check has reached, which is what an - operator has instead. + operator has instead. A domain verified by hand (`domain add --verified`) is the one + exception: it is not listed until a check has passed for it at least once, because + nothing changed for it — a check that then fails, having once passed, is listed again. - **Most link settings in the CLI.** `clickmonk link add` sets targets, a backup URL, a click cap, an expiry, passthrough and traffic action overrides only, and no command changes a link once it is added. Per-device destinations, a returning-visitor @@ -228,7 +230,10 @@ answer 404. Over HTTPS there is no certificate to present, so the connection nev that far: the TLS handshake itself fails, an SSL error with no page behind it. That is what stops somebody else's hostname, pointed at your server, from getting a certificate out of your install. `domain add --verified` is the way round it, for a trial or for a -domain you proved some other way; it says on screen that no DNS check was made. +domain you proved some other way; it says on screen that no DNS check was made, and such +a domain is not shown in `GET /api/alerts` or counted in `GET /api/status` until a check +has passed for it once — until then nothing has changed for it, so there is nothing to +warn about. A check that later fails, having once passed, is an alert again. ClickMonk does not know its own public address, so it records what a domain resolves to rather than judging it — a host behind NAT, a load balancer or a CDN is normal. A domain diff --git a/packages/admin/src/domains.test.ts b/packages/admin/src/domains.test.ts index 6cc75e9..41dae82 100644 --- a/packages/admin/src/domains.test.ts +++ b/packages/admin/src/domains.test.ts @@ -226,6 +226,27 @@ describe('adding a domain', () => { expect(minted.rows.filter((row) => uuidShaped(row.verification_token))).not.toHaveLength(21) }) + // The interface reads `handVerified` rather than re-deriving it, so the + // shape this route answers with is the whole of the contract. + it('answers passedAt and handVerified, computed from verified and the last check', async () => { + const fresh = (await add()).json() + expect(fresh.passedAt).toBeNull() + // Unverified, so not hand-verified either — there is nothing to call + // "verified by hand" about a domain that answers 404. + expect(fresh.handVerified).toBe(false) + + await pg.query( + `INSERT INTO domains (id, host, verified, verification_token) VALUES + ('00000000-0000-4000-8000-0000000000f1', 'byhand.example.test', true, '${'7'.repeat(32)}')`, + ) + const list = await app.inject({ method: 'GET', url: '/api/domains', headers: read(cookie) }) + const byHand = ( + list.json().domains as { host: string; passedAt: unknown; handVerified: unknown }[] + ).find((d) => d.host === 'byhand.example.test') + expect(byHand?.passedAt).toBeNull() + expect(byHand?.handVerified).toBe(true) + }) + it('refuses a body that tries to mark it verified', async () => { expect((await add({ host: 'go.example.test', verified: true })).statusCode).toBe(400) expect((await pg.query('SELECT 1 FROM domains')).rowCount).toBe(0) @@ -413,6 +434,14 @@ describe('checking the DNS on demand', () => { expect(check.rows[0]?.status).toBe('verified') expect(check.rows[0]?.checked_at.toISOString()).toBe(clock.now().toISOString()) expect(resolver.cancelled).toBe(1) + // A check that just passed for the first time: passedAt follows, and the + // domain is no different from one verified by DNS all along. + const after = await app.inject({ method: 'GET', url: '/api/domains', headers: read(cookie) }) + const domain = ( + after.json().domains as { id: string; passedAt: string | null; handVerified: boolean }[] + ).find((d) => d.id === created.id) + expect(domain?.passedAt).toBe(clock.now().toISOString()) + expect(domain?.handVerified).toBe(false) }) it('leaves a domain unverified when the token is not there, and says why', async () => { @@ -603,4 +632,46 @@ describe('what the operator has to know', () => { expect(r.json().domains).toHaveLength(MAX_DOMAINS_LISTED) expect(r.json().truncated).toBe(true) }) + + // The four cases issue #47 draws the line between. Written directly with + // SQL, the way `clickmonk domain add --verified` and a worker pass both + // write: nothing over this API can mark a domain verified or plant a + // `passed_at` of its own choosing. + it('excludes a domain verified by hand until a check has passed for it, and re-alerts it once a passed check later fails', async () => { + await pg.query(`INSERT INTO domains (id, host, verified, verification_token) VALUES + ('00000000-0000-4000-8000-0000000000e1', 'untested.example.test', true, '${'1'.repeat(32)}'), + ('00000000-0000-4000-8000-0000000000e2', 'stillbare.example.test', true, '${'2'.repeat(32)}'), + ('00000000-0000-4000-8000-0000000000e3', 'regressed.example.test', true, '${'3'.repeat(32)}'), + ('00000000-0000-4000-8000-0000000000e4', 'awaiting.example.test', false, '${'4'.repeat(32)}')`) + await pg.query(`INSERT INTO domain_dns_checks (domain_id, status, detail, checked_at, passed_at) VALUES + -- Checked and failed, but never once passed: still hand-verified. + ('00000000-0000-4000-8000-0000000000e2', 'missing_token', 'no record', now(), NULL), + -- Passed once, and its most recent check failed: something changed. + ('00000000-0000-4000-8000-0000000000e3', 'missing_token', 'record gone', now(), '2026-09-01T00:00:00.000Z')`) + // 'untested' has no domain_dns_checks row at all: verified by hand, never checked. + + const alerts = await app.inject({ method: 'GET', url: '/api/alerts', headers: read(cookie) }) + const rows = alerts.json().domains as { + host: string + passedAt: string | null + handVerified: boolean + }[] + const hosts = rows.map((d) => d.host) + expect(hosts).not.toContain('untested.example.test') + expect(hosts).not.toContain('stillbare.example.test') + expect(hosts).toContain('regressed.example.test') + expect(hosts).toContain('awaiting.example.test') + // handVerified is always false here — ALERT_CONDITION already excludes + // every domain it would be true for — but passedAt still carries real + // information: it is what makes 'regressed' an alert at all. + const regressed = rows.find((d) => d.host === 'regressed.example.test') + expect(regressed?.passedAt).toBe('2026-09-01T00:00:00.000Z') + expect(regressed?.handVerified).toBe(false) + const awaiting = rows.find((d) => d.host === 'awaiting.example.test') + expect(awaiting?.passedAt).toBeNull() + expect(awaiting?.handVerified).toBe(false) + + const status = await app.inject({ method: 'GET', url: '/api/status', headers: read(cookie) }) + expect(status.json().alerts).toBe(2) + }) }) diff --git a/packages/admin/src/domains.ts b/packages/admin/src/domains.ts index 8a15418..629be67 100644 --- a/packages/admin/src/domains.ts +++ b/packages/admin/src/domains.ts @@ -44,12 +44,19 @@ import { fail, readBody } from './http.js' export const MAX_DOMAINS_LISTED = 500 /** - * Which domains need attention: every one whose last check did not find the - * token, and every one no check has reached yet. One spelling, read by the - * listing below and by the count `GET /api/status` answers, so "which domains - * need attention" is never asked two ways that could answer differently. + * Which domains need attention: every domain not verified at all, and every + * verified domain whose last check did not find the token — *unless* no + * check has ever passed for it, because then nothing changed: it is exactly + * what `domain add --verified` describes on screen, verified by hand with no + * TXT record, not a domain that stopped proving itself. One spelling, read by + * the listing below and by the count `GET /api/status` answers, so "which + * domains need attention" is never asked two ways that could answer + * differently. */ -export const ALERT_CONDITION = `(c.status IS NULL OR c.status <> 'verified')` +export const ALERT_CONDITION = `( + (c.status IS NULL OR c.status <> 'verified') + AND NOT (d.verified AND c.passed_at IS NULL) +)` /** * How long after a check the same domain may be checked again over the API, @@ -90,10 +97,11 @@ interface DomainRow { status: DomainDnsStatus | null detail: string | null checked_at: Date | null + passed_at: Date | null } const SELECT_DOMAINS = `SELECT d.id, d.host, d.verified, d.root_url, d.not_found_url, - d.verification_token, c.status, c.detail, c.checked_at + d.verification_token, c.status, c.detail, c.checked_at, c.passed_at FROM domains d LEFT JOIN domain_dns_checks c ON c.domain_id = d.id` @@ -115,6 +123,11 @@ function asDomain(d: DomainRow): Record { d.checked_at === null ? null : { status: d.status, detail: d.detail, checkedAt: d.checked_at.toISOString() }, + passedAt: d.passed_at === null ? null : d.passed_at.toISOString(), + // So the interface never re-derives the rule ALERT_CONDITION encodes: a + // domain shown as verified by hand, with nothing to warn about, is one + // that is verified and has never once passed a check. + handVerified: d.verified && d.passed_at === null, } } @@ -268,9 +281,11 @@ export function registerDomainRoutes(app: FastifyInstance, ctx: AdminContext): v /** * What the operator has to know: every domain whose last check did not find - * the token, and every domain no check has reached yet. Verification is - * never revoked automatically, so a domain here may still be serving — that - * is the point of showing it. + * the token, and every domain no check has reached yet — except a domain + * verified by hand that has never once passed a check, which is not shown + * here until it does (`ALERT_CONDITION`). Verification is never revoked + * automatically, so a domain here may still be serving — that is the point + * of showing it. * * Capped and counted exactly as the listing is: an operator shown a prefix * of what is wrong, with no sign that it was a prefix, is worse off than @@ -292,6 +307,12 @@ export function registerDomainRoutes(app: FastifyInstance, ctx: AdminContext): v status: d.status ?? 'never_checked', detail: d.detail, checkedAt: d.checked_at?.toISOString() ?? null, + passedAt: d.passed_at?.toISOString() ?? null, + // Always false here: ALERT_CONDITION already excludes a hand-verified + // domain that has never passed a check, so nothing this route lists + // ever is one. Present anyway, so every domain shape in this API + // carries the same fields. + handVerified: d.verified && d.passed_at === null, })), } }) diff --git a/packages/admin/src/status.test.ts b/packages/admin/src/status.test.ts index a33f000..cd6ce8e 100644 --- a/packages/admin/src/status.test.ts +++ b/packages/admin/src/status.test.ts @@ -82,15 +82,23 @@ beforeAll(async () => { }, ], }) - // Three domains: one verified by a check that found its token, one whose last - // check did not, and one no check has reached. The last two are alerts. + // Five domains, covering every case issue #47 draws the line between: + // d1 checked and currently passing (not an alert, as always); + // d2 verified by hand, no check has reached it yet (not an alert); + // d3 verified by hand, its one check failed, but nothing ever passed (not an alert); + // d4 checked, passed once, and its latest check failed — something changed (an alert); + // d5 not verified at all, awaiting its record (an alert). await pool.query(`INSERT INTO domains (id, host, verified) VALUES ('00000000-0000-4000-8000-0000000000d1', 'ok.example.test', true), - ('00000000-0000-4000-8000-0000000000d2', 'gone.example.test', true), - ('00000000-0000-4000-8000-0000000000d3', 'new.example.test', false)`) - await pool.query(`INSERT INTO domain_dns_checks (domain_id, status, detail, checked_at) VALUES - ('00000000-0000-4000-8000-0000000000d1', 'verified', '', '2026-09-24T11:00:00Z'), - ('00000000-0000-4000-8000-0000000000d2', 'missing_token', 'no TXT record', '2026-09-24T11:00:00Z')`) + ('00000000-0000-4000-8000-0000000000d2', 'untested.example.test', true), + ('00000000-0000-4000-8000-0000000000d3', 'stillbare.example.test', true), + ('00000000-0000-4000-8000-0000000000d4', 'regressed.example.test', true), + ('00000000-0000-4000-8000-0000000000d5', 'new.example.test', false)`) + await pool.query(`INSERT INTO domain_dns_checks (domain_id, status, detail, checked_at, passed_at) VALUES + ('00000000-0000-4000-8000-0000000000d1', 'verified', '', '2026-09-24T11:00:00Z', '2026-09-24T11:00:00Z'), + ('00000000-0000-4000-8000-0000000000d3', 'missing_token', 'no TXT record', '2026-09-24T11:00:00Z', NULL), + ('00000000-0000-4000-8000-0000000000d4', 'missing_token', 'record gone', '2026-09-24T11:00:00Z', '2026-09-01T00:00:00Z')`) + // d2 has no domain_dns_checks row at all: verified by hand, never checked. }) beforeEach(async () => { @@ -129,6 +137,28 @@ describe('GET /api/status', () => { }) }) + // The count alone does not say which two; this pins that it is the right + // two, and that a hand-verified domain's failed check joins them the moment + // something about it changes — a check that once passed. A domain of its + // own, cleaned up after, so the shared fixture the other tests in this file + // read is never left different from what they expect. + it('counts a domain verified by hand as an alert only after a check has passed for it and later failed', async () => { + await pool.query(`INSERT INTO domains (id, host, verified, verification_token) VALUES + ('00000000-0000-4000-8000-0000000000d9', 'handverified.example.test', true, '${'9'.repeat(32)}')`) + await pool.query(`INSERT INTO domain_dns_checks (domain_id, status, detail, checked_at) VALUES + ('00000000-0000-4000-8000-0000000000d9', 'missing_token', 'no record', '2026-09-24T11:00:00Z')`) + try { + expect((await status(app)).json().alerts).toBe(2) + await pool.query( + `UPDATE domain_dns_checks SET passed_at = '2026-09-01T00:00:00Z' + WHERE domain_id = '00000000-0000-4000-8000-0000000000d9'`, + ) + expect((await status(app)).json().alerts).toBe(3) + } finally { + await pool.query(`DELETE FROM domains WHERE id = '00000000-0000-4000-8000-0000000000d9'`) + } + }) + it('says there is no IP data yet, which is not a problem', async () => { const r = await status(app) expect(r.json().ipData).toBeNull() diff --git a/packages/cli/src/commands.test.ts b/packages/cli/src/commands.test.ts index 2a20c02..7b3af6b 100644 --- a/packages/cli/src/commands.test.ts +++ b/packages/cli/src/commands.test.ts @@ -746,6 +746,7 @@ describe('clickmonk domain list and verify', () => { ) expect(r.rows[0]?.verified).toBe(true) expect(lines.join('\n')).toContain('marked verified without a DNS check') + expect(lines.join('\n')).toContain('not shown as needing attention until a check has passed') }) it('verifies a domain whose token is published, and exits 0', async () => { diff --git a/packages/cli/src/commands.ts b/packages/cli/src/commands.ts index 3be5e02..d093dae 100644 --- a/packages/cli/src/commands.ts +++ b/packages/cli/src/commands.ts @@ -181,6 +181,7 @@ async function domainAdd(args: string[], d: CliDeps): Promise { d.out(`domain ${host} ${created.id}`) if (verified) { d.out('marked verified without a DNS check, so it can be given a certificate at once') + d.out('it is not shown as needing attention until a check has passed for it once') return } printVerificationRecords(host, created.verificationToken, d) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index de1861b..07e731e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -2,7 +2,7 @@ * The highest migration version this build understands. Every migration bumps * it; `migrate()` refuses to run against a database whose ledger is newer. */ -export const SCHEMA_VERSION = 9 +export const SCHEMA_VERSION = 10 export * from './admin-host.js' export * from './attempts.js' diff --git a/packages/core/src/version.test.ts b/packages/core/src/version.test.ts index fde79a1..8dac57d 100644 --- a/packages/core/src/version.test.ts +++ b/packages/core/src/version.test.ts @@ -5,6 +5,6 @@ describe('SCHEMA_VERSION', () => { it('is the highest migration version this build knows about', () => { // Bumped with every migration added. packages/db's // schema-version.test.ts ties it to the migration files on disk. - expect(SCHEMA_VERSION).toBe(9) + expect(SCHEMA_VERSION).toBe(10) }) }) diff --git a/packages/db/migrations/postgres/010_domain_check_passed_at.sql b/packages/db/migrations/postgres/010_domain_check_passed_at.sql new file mode 100644 index 0000000..e0c30de --- /dev/null +++ b/packages/db/migrations/postgres/010_domain_check_passed_at.sql @@ -0,0 +1,20 @@ +-- When a domain's DNS check last passed, so a domain verified by hand (no +-- check has ever found its token) can be told apart from one that was +-- checked and found wanting. `domains.verified` alone cannot make that +-- distinction: it is set by a passing check or by `domain add --verified`, +-- and never records which. +-- +-- Only `domain_dns_checks` is altered here, for the same reason 005 put the +-- check result in its own table rather than on `domains`: the redirect's +-- snapshot load reads `links` and then `domains` in one transaction, and a +-- migration holding a lock on both, in the other order, could deadlock +-- against it and be the statement Postgres aborts. This migration takes an +-- ACCESS EXCLUSIVE lock on `domain_dns_checks` alone, a table the redirect's +-- snapshot load never reads, so it cannot be either half of that deadlock. +-- +-- The column is added without a default, backfilled, and left without one: +-- every future row is written by `recordDomainCheck`'s single upsert, which +-- always states `passed_at` explicitly, so a default would only paper over a +-- caller that forgot to. +ALTER TABLE domain_dns_checks ADD COLUMN passed_at timestamptz NULL; +UPDATE domain_dns_checks SET passed_at = checked_at WHERE status = 'verified'; diff --git a/packages/db/src/migration-010.test.ts b/packages/db/src/migration-010.test.ts new file mode 100644 index 0000000..3b95689 --- /dev/null +++ b/packages/db/src/migration-010.test.ts @@ -0,0 +1,61 @@ +import { afterAll, describe, expect, it } from 'vitest' +import { MIGRATIONS_DIR } from './migrate-latest.js' +import { loadMigrations, migrate } from './migrator.js' +import { testCh, testPg } from './testing.js' + +const pg = testPg() +const ch = testCh() + +/** + * Replays a real upgrade: a database migrated only as far as 009, carrying + * checks from before `passed_at` existed, then brought up to 010. This is + * the one case `resetDatabases` (migrate-from-nothing, used by every other + * test file) cannot exercise, and the one migration 010 is about: a domain + * whose last check already said 'verified' before this column existed must + * come out the upgrade with `passed_at` backfilled, not null. + */ +describe('migration 010: domain_dns_checks.passed_at', () => { + afterAll(async () => { + await pg.end() + await ch.close() + }) + + it('adds the column, backfills a passed check, and leaves a failed one null', async () => { + await pg.query('DROP SCHEMA public CASCADE; CREATE SCHEMA public;') + const rs = await ch.query({ query: 'SHOW TABLES', format: 'JSONEachRow' }) + for (const { name } of await rs.json<{ name: string }>()) { + await ch.command({ query: `DROP TABLE IF EXISTS \`${name.replace(/`/g, '')}\`` }) + } + + const all = loadMigrations(MIGRATIONS_DIR) + const upTo9 = all.filter((m) => m.version <= 9) + expect(upTo9.length).toBeLessThan(all.length) + await migrate({ pg, ch, migrations: upTo9, appSchemaVersion: 9 }) + + // A pre-010 database: two domains, one whose last check found the token + // and one whose last check did not — written with the columns 009 knew + // about, exactly as an installed database would have them. + await pg.query(`INSERT INTO domains (id, host, verification_token) VALUES + ('00000000-0000-4000-8000-000000000a01', 'passed.example.test', '${'a'.repeat(32)}'), + ('00000000-0000-4000-8000-000000000a02', 'failed.example.test', '${'b'.repeat(32)}')`) + await pg.query(`INSERT INTO domain_dns_checks (domain_id, status, detail, checked_at) VALUES + ('00000000-0000-4000-8000-000000000a01', 'verified', 'ok', '2026-09-01T00:00:00.000Z'), + ('00000000-0000-4000-8000-000000000a02', 'missing_token', 'no record', '2026-09-02T00:00:00.000Z')`) + + const result = await migrate({ pg, ch, migrations: all, appSchemaVersion: 10 }) + expect(result.applied).toEqual([10]) + + const rows = await pg.query<{ host: string; passed_at: Date | null; checked_at: Date }>( + `SELECT d.host, c.passed_at, c.checked_at + FROM domain_dns_checks c JOIN domains d ON d.id = c.domain_id + ORDER BY d.host`, + ) + const passed = rows.rows.find((r) => r.host === 'passed.example.test') + const failed = rows.rows.find((r) => r.host === 'failed.example.test') + expect(passed?.passed_at?.toISOString()).toBe(passed?.checked_at.toISOString()) + expect(failed?.passed_at).toBeNull() + + // Applying it again is a no-op: the ledger already has 010. + expect((await migrate({ pg, ch, migrations: all, appSchemaVersion: 10 })).applied).toEqual([]) + }) +}) diff --git a/packages/ui/src/api/types.ts b/packages/ui/src/api/types.ts index f612f42..88c14d2 100644 --- a/packages/ui/src/api/types.ts +++ b/packages/ui/src/api/types.ts @@ -63,6 +63,10 @@ export interface Domain { notFoundUrl: string | null verificationRecord: { name: string; type: 'TXT'; value: string } lastCheck: { status: DomainStatus; detail: string | null; checkedAt: Instant } | null + /** When a check last found the token, or never has. */ + passedAt: Instant | null + /** `verified && passedAt === null`: verified by hand, with no check having passed for it yet. */ + handVerified: boolean } export interface DomainCheck { diff --git a/packages/ui/src/screens/domains/DomainCard.tsx b/packages/ui/src/screens/domains/DomainCard.tsx index fc70049..a932b81 100644 --- a/packages/ui/src/screens/domains/DomainCard.tsx +++ b/packages/ui/src/screens/domains/DomainCard.tsx @@ -113,7 +113,9 @@ export function DomainCard({ d, onChanged }: { d: Domain; onChanged: () => void {/* The record, prominent while it is still needed, and always available. */}

- {d.verified ? 'The record that proved it:' : 'Publish this TXT record, then check:'} + {d.verified && !d.handVerified + ? 'The record that proved it:' + : 'Publish this TXT record, then check:'}

Name
@@ -132,7 +134,27 @@ export function DomainCard({ d, onChanged }: { d: Domain; onChanged: () => void
- {check ? ( + {/* Hand-verified outranks a check just run in this session: a + check on a hand-verified domain does not change the badge or + this framing, and `onChanged()` replaces `check` with the + reload's own `lastCheck` moments later regardless — showing + the raw result in between would only flash and vanish. The + check's own words and time are shown here too, muted, so that + running one still leaves something durable to see. */} + {d.handVerified ? ( + <> +

Verified by hand, no TXT record

+ {d.lastCheck && ( +

+ {d.lastCheck.detail + ? `${CHECK_WORDS[d.lastCheck.status]}: ${d.lastCheck.detail}` + : CHECK_WORDS[d.lastCheck.status]} + {' — '} + {formatInstant(d.lastCheck.checkedAt, zone)} +

+ )} + + ) : check ? (

{check.detail ? `${CHECK_WORDS[check.status]}: ${check.detail}` diff --git a/packages/ui/src/screens/domains/Domains.test.tsx b/packages/ui/src/screens/domains/Domains.test.tsx index 6999e7f..711d56b 100644 --- a/packages/ui/src/screens/domains/Domains.test.tsx +++ b/packages/ui/src/screens/domains/Domains.test.tsx @@ -26,6 +26,8 @@ const domain = (host: string, over: Partial = {}): Domain => ({ notFoundUrl: null, verificationRecord: { name: `_clickmonk.${host}`, type: 'TXT', value: TOKEN }, lastCheck: null, + passedAt: null, + handVerified: false, ...over, }) @@ -114,6 +116,60 @@ describe('the domain list', () => { expect(within(c).getByText('no TXT record at _clickmonk.go.example.test')).toBeInTheDocument() }) + it('says a hand-verified domain has no TXT record, in place of its failing check, and still badges it Verified', async () => { + show([ + domain('go.example.test', { + verified: true, + handVerified: true, + lastCheck: { + status: 'missing_token', + detail: 'no TXT record at _clickmonk.go.example.test', + checkedAt: '2026-10-07T02:00:00.000Z', + }, + }), + ]) + const c = await card('go.example.test') + expect(within(c).getByText('Verified')).toBeInTheDocument() + expect(within(c).getByText('Verified by hand, no TXT record')).toBeInTheDocument() + expect(within(c).queryByText('Record not found')).not.toBeInTheDocument() + // The card is not claiming DNS proved anything — the label above the + // record says to publish it, the same as an unverified domain's does. + expect(within(c).getByText('Publish this TXT record, then check:')).toBeInTheDocument() + expect(within(c).queryByText('The record that proved it:')).not.toBeInTheDocument() + // The record to publish is still there, unaffected. + expect(within(c).getByText('_clickmonk.go.example.test')).toBeInTheDocument() + // The last check's own result is not lost — shown under the hand-verified + // line, in that order, muted rather than framed as a problem. + const handVerifiedLine = within(c).getByText('Verified by hand, no TXT record') + const lastCheckLine = within(c).getByText(/no TXT record at _clickmonk\.go\.example\.test/) + expect( + handVerifiedLine.compareDocumentPosition(lastCheckLine) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy() + }) + + it('keeps the hand-verified framing even right after Check now finds nothing, rather than flashing the raw result', async () => { + const { user } = show( + [ + domain('go.example.test', { + verified: true, + handVerified: true, + lastCheck: { + status: 'missing_token', + detail: 'no TXT record at _clickmonk.go.example.test', + checkedAt: '2026-10-07T02:00:00.000Z', + }, + }), + ], + { checkDomain: () => Promise.resolve({ status: 'missing_token', detail: 'still nothing' }) }, + ) + const c = await card('go.example.test') + await user.click(within(c).getByRole('button', { name: 'Check now' })) + // Neither the un-muted, un-framed rendering `check` alone would produce + // nor a bare "Record not found" ever appears: hand-verified outranks it. + expect(within(c).getByText('Verified by hand, no TXT record')).toBeInTheDocument() + expect(within(c).queryByText('Record not found: still nothing')).not.toBeInTheDocument() + }) + it('offers no way to mark a domain verified', async () => { show([domain('go.example.test')]) await card('go.example.test') diff --git a/packages/ui/src/screens/links/LinkForm.test.tsx b/packages/ui/src/screens/links/LinkForm.test.tsx index f932a63..9146bba 100644 --- a/packages/ui/src/screens/links/LinkForm.test.tsx +++ b/packages/ui/src/screens/links/LinkForm.test.tsx @@ -20,6 +20,8 @@ const domain = (host: string, verified: boolean): Domain => ({ value: 'clickmonk-verify=0123456789abcdef0123456789abcdef', }, lastCheck: null, + passedAt: null, + handVerified: false, }) const SETTINGS: Settings = { diff --git a/packages/ui/src/screens/links/Links.test.tsx b/packages/ui/src/screens/links/Links.test.tsx index 1144728..c784f82 100644 --- a/packages/ui/src/screens/links/Links.test.tsx +++ b/packages/ui/src/screens/links/Links.test.tsx @@ -20,6 +20,8 @@ const domain = (host: string, verified: boolean): Domain => ({ value: 'clickmonk-verify=0123456789abcdef0123456789abcdef', }, lastCheck: null, + passedAt: null, + handVerified: false, }) const link = (slug: string, host = 'go.example.test', over: Partial = {}): Link => ({ diff --git a/packages/worker/src/domains.test.ts b/packages/worker/src/domains.test.ts index f763344..1fa36fc 100644 --- a/packages/worker/src/domains.test.ts +++ b/packages/worker/src/domains.test.ts @@ -222,8 +222,8 @@ async function addDomain( const checkOf = (id: string) => pool - .query<{ status: string; detail: string; checked_at: Date }>( - 'SELECT status, detail, checked_at FROM domain_dns_checks WHERE domain_id = $1', + .query<{ status: string; detail: string; checked_at: Date; passed_at: Date | null }>( + 'SELECT status, detail, checked_at, passed_at FROM domain_dns_checks WHERE domain_id = $1', [id], ) .then((r) => r.rows[0]) @@ -353,27 +353,111 @@ describe('recordDomainCheck', () => { it('writes the check and marks the domain verified the first time its token is found', async () => { const d = await addDomain('go.example.test') + const now = new Date() await recordDomainCheck( pool, { id: d.id, verified: false }, { status: 'verified', detail: 'token found' }, - new Date(), + now, ) expect(await verifiedOf(d.id)).toBe(true) expect((await checkOf(d.id))?.status).toBe('verified') expect((await checkOf(d.id))?.detail).toBe('token found') - }) + // A pass stamps passed_at with this check's own time. + expect((await checkOf(d.id))?.passed_at?.toISOString()).toBe(now.toISOString()) + }) + + // 'error' matters here as much as 'missing_token': only 'verified' counts + // as a pass, so a resolver outage on a hand-verified domain must not stamp + // passed_at either — that would make the *next* failing check an alert, + // which is #47 again for a domain that never actually proved itself. + it.each(['missing_token', 'error'] as const)( + 'records a failed check (%s) without un-verifying a domain that already was, and leaves passed_at null', + async (status) => { + const d = await addDomain('go.example.test', true) + await recordDomainCheck( + pool, + { id: d.id, verified: true }, + { status, detail: 'no TXT record' }, + new Date(), + ) + expect(await verifiedOf(d.id)).toBe(true) + expect((await checkOf(d.id))?.status).toBe(status) + // Verified by hand, and this is its first check: nothing has ever passed. + expect((await checkOf(d.id))?.passed_at).toBeNull() + }, + ) - it('records a failed check without un-verifying a domain that already was', async () => { - const d = await addDomain('go.example.test', true) + it.each(['missing_token', 'error'] as const)( + 'keeps passed_at at the time of the last pass when a later check %s', + async (status) => { + const d = await addDomain('go.example.test') + const passedAt = new Date('2026-09-20T00:00:00.000Z') + await recordDomainCheck( + pool, + { id: d.id, verified: false }, + { status: 'verified', detail: 'ok' }, + passedAt, + ) + await recordDomainCheck( + pool, + { id: d.id, verified: true }, + { status, detail: 'record gone' }, + new Date('2026-09-21T00:00:00.000Z'), + ) + const row = await checkOf(d.id) + expect(row?.status).toBe(status) + expect(row?.passed_at?.toISOString()).toBe(passedAt.toISOString()) + }, + ) + + it('stamps passed_at on a later check that passes, updating an existing row rather than only an inserted one', async () => { + const d = await addDomain('go.example.test') + await recordDomainCheck( + pool, + { id: d.id, verified: false }, + { status: 'missing_token', detail: 'no record yet' }, + new Date('2026-09-20T00:00:00.000Z'), + ) + expect((await checkOf(d.id))?.passed_at).toBeNull() + const passedAt = new Date('2026-09-21T00:00:00.000Z') + await recordDomainCheck( + pool, + { id: d.id, verified: false }, + { status: 'verified', detail: 'ok' }, + passedAt, + ) + const row = await checkOf(d.id) + expect(row?.status).toBe('verified') + expect(row?.passed_at?.toISOString()).toBe(passedAt.toISOString()) + }) + + // The test above restamps from null, so `COALESCE`'s argument order cannot + // tell "take this check's stamp" from "take the row's own" apart — either + // order picks the one non-null value. Only a *second* pass, after an + // earlier one, has both sides non-null and different, and only then does + // getting the order backwards show up: it would leave passed_at stuck at + // the first pass forever, contradicting "when a check last passed". + it('restamps passed_at to a later pass, not the first one', async () => { + const d = await addDomain('go.example.test') + const t1 = new Date('2026-09-20T00:00:00.000Z') + await recordDomainCheck( + pool, + { id: d.id, verified: false }, + { status: 'verified', detail: 'ok' }, + t1, + ) + expect((await checkOf(d.id))?.passed_at?.toISOString()).toBe(t1.toISOString()) + const t2 = new Date('2026-09-25T00:00:00.000Z') await recordDomainCheck( pool, { id: d.id, verified: true }, - { status: 'missing_token', detail: 'no TXT record' }, - new Date(), + { status: 'verified', detail: 'still ok' }, + t2, ) - expect(await verifiedOf(d.id)).toBe(true) - expect((await checkOf(d.id))?.status).toBe('missing_token') + const row = await checkOf(d.id) + expect(row?.status).toBe('verified') + expect(row?.passed_at?.toISOString()).toBe(t2.toISOString()) }) it('replaces an existing check row rather than duplicating it', async () => { diff --git a/packages/worker/src/domains.ts b/packages/worker/src/domains.ts index 2f08739..3acbc36 100644 --- a/packages/worker/src/domains.ts +++ b/packages/worker/src/domains.ts @@ -280,6 +280,21 @@ export const DEFAULT_CHECK_LIMIT = 50 * redirect reload its whole configuration for nothing. A domain is never * un-verified here — a resolver outage or a DNS edit must not take live * links down or stop a certificate renewing. + * + * `passed_at` is stamped with `now` on a pass and left exactly as it was on a + * failure — the same "never taken back" rule `verified` itself follows, and + * for the same reason: a resolver hiccup must not turn a domain that once + * proved itself back into one that has not. Whether *this* check passed is + * decided exactly once, in JS (`passedAt`), rather than repeating + * `result.status` as a second bound parameter inside the statement — + * Postgres cannot always agree with itself on one placeholder's type across + * two different positions in the same query, and this sidesteps that rather + * than fighting it with casts. The upsert then only has to choose between + * that decision and the row's own history: `COALESCE(EXCLUDED.passed_at, + * domain_dns_checks.passed_at)` takes this check's stamp when it passed + * (`EXCLUDED.passed_at` is non-null only then) and otherwise falls through to + * whatever the row already had — never re-deciding "did this pass" a second + * time in SQL. */ export async function recordDomainCheck( pg: Pool, @@ -287,12 +302,16 @@ export async function recordDomainCheck( result: DomainCheck, now: Date, ): Promise { + const passedAt = result.status === 'verified' ? now : null await pg.query( - `INSERT INTO domain_dns_checks (domain_id, status, detail, checked_at) - VALUES ($1, $2, $3, $4) + `INSERT INTO domain_dns_checks (domain_id, status, detail, checked_at, passed_at) + VALUES ($1, $2, $3, $4, $5) ON CONFLICT (domain_id) DO UPDATE - SET status = EXCLUDED.status, detail = EXCLUDED.detail, checked_at = EXCLUDED.checked_at`, - [row.id, result.status, result.detail, now], + SET status = EXCLUDED.status, + detail = EXCLUDED.detail, + checked_at = EXCLUDED.checked_at, + passed_at = COALESCE(EXCLUDED.passed_at, domain_dns_checks.passed_at)`, + [row.id, result.status, result.detail, now, passedAt], ) if (result.status === 'verified' && !row.verified) { await pg.query('UPDATE domains SET verified = true, updated_at = $2 WHERE id = $1', [