diff --git a/.github/workflows/content-pipeline-watchdog.yml b/.github/workflows/content-pipeline-watchdog.yml index c230a0c79..b3aad9a5e 100644 --- a/.github/workflows/content-pipeline-watchdog.yml +++ b/.github/workflows/content-pipeline-watchdog.yml @@ -49,11 +49,15 @@ jobs: RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | set -uo pipefail - - # Every API call is `|| true`: the step shell is `bash -e`, so an - # unguarded 403 aborts mid-script and surfaces as a bare red run - # indistinguishable from a real content alert. That happened once - # (actions:read 403). Collect what we can, then decide explicitly. + # `set +e` is deliberate, and `-e` here is actively harmful. + # GitHub runs this step as `bash -e {0}`, which combined with + # pipefail means ANY non-zero anywhere kills the script at that + # line with no output at all — a red run and an empty log. It bit + # twice: a 403 from the Actions API, then a `grep` with no match + # inside the mirrored-paths filter (grep exits 1 on no-match, which + # is a normal answer, not an error). This script decides everything + # explicitly below; it must never die silently mid-check. + set +e # ---- hop 3: what production is serving ----------------------- LIVE=$(GH_TOKEN=$UI_TOKEN gh api "repos/$REPO/contents/src/content?ref=main" --jq '.sha' || true) @@ -87,6 +91,15 @@ jobs: if [ -n "$PUBLISHED" ]; then EXPECTED="$SHA"; break; fi done + # On failure `gh` prints its error body to stdout, which would land + # in these vars and read as a bogus sha. Keep only real ones, so a + # broken call resolves to empty and trips the DEGRADED branch below + # instead of being compared as if it were data. + only_sha() { printf '%s' "${1:-}" | grep -Eox '[0-9a-f]{40}' || true; } + LIVE=$(only_sha "$LIVE") + TIP=$(only_sha "$TIP") + EXPECTED=$(only_sha "$EXPECTED") + MIRRORED=$(printf '%s' "$TIP_MSG" | sed -nE 's/.*mono@([0-9a-f]+).*/\1/p') echo "expected mono@${EXPECTED:0:7} | mirrored @${MIRRORED:-none} | peanut-content ${TIP:0:7} | live ${LIVE:0:7}" diff --git a/.github/workflows/content-publish-automerge.yml b/.github/workflows/content-publish-automerge.yml index ecac7aab4..846e63754 100644 --- a/.github/workflows/content-publish-automerge.yml +++ b/.github/workflows/content-publish-automerge.yml @@ -102,7 +102,13 @@ jobs: SHA: ${{ github.event.workflow_run.head_sha }} REPO: ${{ github.repository }} run: | - set -euo pipefail + set -uo pipefail + # `set +e` deliberately. GitHub runs this as `bash -e {0}`, so any + # unguarded non-zero — a transient `gh` 5xx, a PR that closed between + # the list and the diff — kills the step mid-loop with no output and + # a bare red run. Every branch below checks its own inputs and says + # what it decided; dying silently is strictly worse than continuing. + set +e # Key off the `ci-success` check run, NOT `workflow_run.conclusion`. # `ci-success` is the single check the "Protect Prod" ruleset gates @@ -115,8 +121,15 @@ jobs: # carries TWO ci-success check runs. Require at least one and ALL of # them green — a single red run must block the merge. RUNS="repos/$REPO/commits/$SHA/check-runs?per_page=100" - TOTAL=$(gh api "$RUNS" --jq '[.check_runs[] | select(.name=="ci-success")] | length') - GREEN=$(gh api "$RUNS" --jq '[.check_runs[] | select(.name=="ci-success" and .conclusion=="success")] | length') + TOTAL=$(gh api "$RUNS" --jq '[.check_runs[] | select(.name=="ci-success")] | length' || echo 0) + GREEN=$(gh api "$RUNS" --jq '[.check_runs[] | select(.name=="ci-success" and .conclusion=="success")] | length' || echo 0) + # On failure `gh` prints its JSON error body to stdout, which lands + # in these vars. Without this, `[ "$TOTAL" -eq 0 ]` errors out and the + # loop below iterates over the error text as if the words were PR + # numbers. Anything non-numeric means "we don't know" -> don't merge. + case "$TOTAL" in '' | *[!0-9]*) TOTAL=0 ;; esac + case "$GREEN" in '' | *[!0-9]*) GREEN=-1 ;; esac + if [ "$TOTAL" -eq 0 ] || [ "$TOTAL" -ne "$GREEN" ]; then echo "ci-success not green for $SHA ($GREEN/$TOTAL) — nothing to merge." exit 0 @@ -140,12 +153,35 @@ jobs: fi for PR in $PRS; do - FILES=$(gh pr diff "$PR" --repo "$REPO" --name-only) + # Same reason: never treat a fragment of an error body as a PR number. + case "$PR" in '' | *[!0-9]*) echo "::warning::ignoring non-numeric PR id '$PR'"; continue ;; esac + FILES=$(gh pr diff "$PR" --repo "$REPO" --name-only || echo UNKNOWN) if [ "$FILES" != "src/content" ]; then echo "::notice::#$PR is not content-only — normal review gate stays in place." continue fi echo "Merging #$PR — diff is exactly src/content, ci-success green ($GREEN/$TOTAL) on $SHA." - gh pr merge "$PR" --repo "$REPO" --merge \ - || echo "::warning::merge failed for #$PR (already merged, or bypass unavailable) — merge manually." + # `--admin` is required, not optional. Being an OrganizationAdmin + # does NOT make `gh pr merge` bypass "Protect Prod" — gh enforces + # the branch policy and tells you to pass --admin explicitly. + # Without it every merge here fails with "the base branch policy + # prohibits the merge", which is exactly what happened to #2547. + # + # --admin also skips the required status check, so note what it is + # NOT skipping: this job has already established, itself, that the + # diff is exactly src/content and that every ci-success run on this + # commit is green. The gate is enforced above, in code, before we + # get here — --admin only waives the human review, which is the + # whole point of this workflow. + if ! MERGE_ERR=$(gh pr merge "$PR" --repo "$REPO" --merge --admin 2>&1); then + # A merge we decided to make and then failed to make is the exact + # silent failure this pipeline exists to prevent. Be loud. + echo "::error::MERGE FAILED for #$PR — content will NOT reach production. $MERGE_ERR" + FAILED=1 + fi done + + if [ "${FAILED:-0}" = "1" ]; then + echo "At least one content publish could not be merged (see errors above)." + exit 1 + fi diff --git a/src/content b/src/content index c268e5d9c..6ad000619 160000 --- a/src/content +++ b/src/content @@ -1 +1 @@ -Subproject commit c268e5d9c436ae742915b26873e4066748883acc +Subproject commit 6ad00061928298ea34cf0a76f5a91ef9d1dc2b42 diff --git a/src/hooks/__tests__/usePrimaryNameServer.test.tsx b/src/hooks/__tests__/usePrimaryNameServer.test.tsx index bf0aad773..ce148a5f5 100644 --- a/src/hooks/__tests__/usePrimaryNameServer.test.tsx +++ b/src/hooks/__tests__/usePrimaryNameServer.test.tsx @@ -1,12 +1,15 @@ import { renderHook, waitFor } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import type { ReactNode } from 'react' +import { usePrimaryName } from '@justaname.id/react' import { usePrimaryNameServer } from '../usePrimaryNameServer' import { serverFetch } from '@/utils/api-fetch' jest.mock('@/utils/api-fetch', () => ({ serverFetch: jest.fn() })) const mockServerFetch = serverFetch as jest.MockedFunction +// resolves to the shared manual mock via jest moduleNameMapper +const mockUsePrimaryName = usePrimaryName as jest.Mock const wrapper = ({ children }: { children: ReactNode }) => { const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) @@ -17,7 +20,10 @@ const ADDRESS = '0x1bbb000000000000000000000000000000ea0349' const jsonResponse = (body: unknown, ok = true) => ({ ok, json: async () => body }) as unknown as Response -beforeEach(() => jest.clearAllMocks()) +beforeEach(() => { + jest.clearAllMocks() + mockUsePrimaryName.mockReturnValue({ primaryName: undefined, isLoading: false, error: null }) +}) describe('usePrimaryNameServer', () => { it('resolves the address to its primary name via the backend', async () => { @@ -25,19 +31,35 @@ describe('usePrimaryNameServer', () => { const { result } = renderHook(() => usePrimaryNameServer(ADDRESS), { wrapper }) await waitFor(() => expect(result.current.primaryName).toBe('alice.eth')) expect(mockServerFetch).toHaveBeenCalledWith(`/ens/reverse/${ADDRESS}`, { method: 'GET' }) + // client fallback stays a no-op while the server path works + expect(mockUsePrimaryName).toHaveBeenLastCalledWith(expect.objectContaining({ address: undefined })) }) it('returns undefined when the backend reports no name', async () => { - mockServerFetch.mockResolvedValue(jsonResponse({ name: null })) + const json = jest.fn(async () => ({ name: null })) + mockServerFetch.mockResolvedValue({ ok: true, json } as unknown as Response) const { result } = renderHook(() => usePrimaryNameServer(ADDRESS), { wrapper }) - await waitFor(() => expect(mockServerFetch).toHaveBeenCalled()) + // wait for the resolved-null query to re-render the hook, not just for the fetch call + await waitFor(() => expect(mockUsePrimaryName.mock.calls.length).toBeGreaterThan(1)) expect(result.current.primaryName).toBeUndefined() + // a live endpoint answering "no name" is authoritative — client fallback stays disabled + expect(mockUsePrimaryName).toHaveBeenLastCalledWith(expect.objectContaining({ address: undefined })) + }) + + it('falls back to the client-side lookup on a non-OK response (e.g. route not deployed)', async () => { + mockServerFetch.mockResolvedValue(jsonResponse({}, false)) + mockUsePrimaryName.mockReturnValue({ primaryName: 'alice.eth', isLoading: false, error: null }) + const { result } = renderHook(() => usePrimaryNameServer(ADDRESS), { wrapper }) + await waitFor(() => expect(result.current.primaryName).toBe('alice.eth')) + expect(mockUsePrimaryName).toHaveBeenLastCalledWith(expect.objectContaining({ address: ADDRESS })) }) - it('fails safe to undefined on a non-OK response (e.g. route not deployed)', async () => { + it('fails safe to undefined when both server and client lookups fail', async () => { mockServerFetch.mockResolvedValue(jsonResponse({}, false)) const { result } = renderHook(() => usePrimaryNameServer(ADDRESS), { wrapper }) - await waitFor(() => expect(mockServerFetch).toHaveBeenCalled()) + await waitFor(() => + expect(mockUsePrimaryName).toHaveBeenLastCalledWith(expect.objectContaining({ address: ADDRESS })) + ) expect(result.current.primaryName).toBeUndefined() }) diff --git a/src/hooks/usePrimaryNameServer.ts b/src/hooks/usePrimaryNameServer.ts index cc5c1d718..ec93fec9b 100644 --- a/src/hooks/usePrimaryNameServer.ts +++ b/src/hooks/usePrimaryNameServer.ts @@ -1,5 +1,6 @@ 'use client' +import { usePrimaryName } from '@justaname.id/react' import { useQuery } from '@tanstack/react-query' import { isAddress } from 'viem' import { serverFetch } from '@/utils/api-fetch' @@ -7,36 +8,49 @@ import { serverFetch } from '@/utils/api-fetch' const PRIMARY_NAME_TTL_MS = 24 * 60 * 60 * 1000 // 1 day /** - * ENS reverse-lookup (address → primary name) resolved SERVER-SIDE via the - * backend `/ens/reverse` endpoint, instead of a client call to the mainnet RPC. + * ENS reverse-lookup (address → primary name), server-first with a client-side + * fallback. * - * Why: on native the WebView origin (`capacitor://localhost` / `https://localhost`) - * is rejected by the RPC's CORS policy, so the client-side `usePrimaryName` - * (JustaName, `priority: 'onChain'`) always failed on mobile and the feed fell - * back to the raw address. The backend has no such origin restriction and is - * already CORS-allowed for native, so this resolves identically on web and - * native. Drop-in for `@justaname.id/react`'s `usePrimaryName` — same - * `{ primaryName }` shape. + * Server-first: the backend `/ens/reverse` endpoint works on native, where the + * WebView origin (`capacitor://localhost` / `https://localhost`) is rejected by + * the mainnet RPC's CORS policy and a client lookup can never succeed. * - * Fail-safe: any non-OK response (incl. a backend that doesn't yet expose the - * route) yields `undefined`, so callers degrade to the address exactly as before. + * Client fallback: the endpoint is not deployed everywhere yet (peanut-api-ts + * #1237). When the server call fails, fall back to JustaName's client-side + * on-chain lookup — this keeps ENS names working on web today, and native + * picks up the server path automatically the moment the endpoint ships, + * with no frontend change. + * + * Drop-in for `@justaname.id/react`'s `usePrimaryName` — same + * `{ primaryName }` shape. If both paths fail, callers degrade to the raw + * address exactly as before. */ export function usePrimaryNameServer(address?: string): { primaryName: string | undefined } { const enabled = !!address && isAddress(address) - const { data } = useQuery({ + const { data, isError } = useQuery({ queryKey: ['ens-primary-name', address?.toLowerCase()], enabled, staleTime: PRIMARY_NAME_TTL_MS, gcTime: PRIMARY_NAME_TTL_MS, retry: false, queryFn: async (): Promise => { + // throw on non-OK so the client fallback below engages, instead of + // caching a negative result for 24h while the route 404s const res = await serverFetch(`/ens/reverse/${address}`, { method: 'GET' }) - if (!res.ok) return null + if (!res.ok) throw new Error(`ens reverse lookup failed: ${res.status}`) const json = (await res.json()) as { name?: string | null } return json.name ?? null }, }) - return { primaryName: data ?? undefined } + // fallback lookup, only engaged when the server call failed. address stays + // undefined otherwise, which makes the hook a no-op. + const { primaryName: clientName } = usePrimaryName({ + address: enabled && isError ? (address as `0x${string}`) : undefined, + chainId: 1, // mainnet for ens lookups + priority: 'onChain', + }) + + return { primaryName: data ?? (isError ? clientName : undefined) } }