From 5ead32a90f7435ac58d83c41bf148d2d6bc35907 Mon Sep 17 00:00:00 2001 From: Gil Desmarais Date: Fri, 28 Aug 2026 23:32:05 +0200 Subject: [PATCH 1/2] fix(web): fail-closed BLOCKED_SURFACE copy and chrome Honest Decision sentence for blocked surfaces (try another URL/site) and shared non-retryable create title without false-hope "yet"; retryable timeouts keep Try again chrome. Wire body stays Decision SSOT. --- app/web/errors/error_classifier.rb | 2 +- frontend/e2e/smoke.spec.ts | 11 ++++++----- frontend/src/__tests__/App.test.tsx | 16 +++++++++++----- frontend/src/__tests__/ResultDisplay.test.tsx | 6 ++++-- frontend/src/__tests__/previewHydration.test.ts | 4 ++-- frontend/src/components/AppPanels.tsx | 2 +- frontend/src/journey/copy.ts | 5 ++++- spec/html2rss/web/error_classifier_spec.rb | 4 +++- 8 files changed, 32 insertions(+), 18 deletions(-) diff --git a/app/web/errors/error_classifier.rb b/app/web/errors/error_classifier.rb index ad98e132..8d604262 100644 --- a/app/web/errors/error_classifier.rb +++ b/app/web/errors/error_classifier.rb @@ -130,7 +130,7 @@ def initialize(decision) ).freeze BLOCKED_SURFACE_CODE = 'BLOCKED_SURFACE' - BLOCKED_SURFACE_MESSAGE = 'This website blocked automated access.' + BLOCKED_SURFACE_MESSAGE = 'This site blocked automated access. Try another URL or site.' BLOCKED_SURFACE = Decision.new( status: 422, diff --git a/frontend/e2e/smoke.spec.ts b/frontend/e2e/smoke.spec.ts index b4a8ae3d..51c31868 100644 --- a/frontend/e2e/smoke.spec.ts +++ b/frontend/e2e/smoke.spec.ts @@ -41,7 +41,8 @@ test.describe('frontend smoke', () => { await expect(page.getByRole('button', { name: COPY.back })).toBeVisible(); await expect(page.locator('dialog')).toHaveAttribute('open'); await expect(page.getByLabel(COPY.urlLabel)).toHaveCount(1); - await expect(page.getByText(COPY.createFailedTitle)).toHaveCount(0); + await expect(page.getByText(COPY.createFailedTitle, { exact: true })).toHaveCount(0); + await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toHaveCount(0); await page.getByRole('button', { name: COPY.back }).click(); await expect(page).toHaveURL(/#\/create(?:\?.*)?$/); @@ -96,21 +97,21 @@ test.describe('frontend smoke', () => { await page.getByLabel(COPY.urlLabel).fill('https://example.com/articles'); await page.getByRole('button', { name: COPY.createFeed }).click(); - await expect(page.getByText(COPY.createFailedTitle)).toBeVisible(); + await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toBeVisible(); await page.getByRole('link', { name: 'html2rss' }).click(); - await expect(page.getByText(COPY.createFailedTitle)).toHaveCount(0); + await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toHaveCount(0); await expect(page.locator('.form-shell')).toHaveAttribute('data-state', 'create'); await expect(page.getByLabel(COPY.urlLabel)).toBeFocused(); await page.getByRole('button', { name: COPY.createFeed }).click(); - await expect(page.getByText(COPY.createFailedTitle)).toBeVisible(); + await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toBeVisible(); await page.evaluate(() => { location.hash = '#!/create'; }); await expect(page).toHaveURL(/\/#\/create$/); - await expect(page.getByText(COPY.createFailedTitle)).toHaveCount(0); + await expect(page.getByText(COPY.createFailedRetryTitle, { exact: true })).toHaveCount(0); await expect(page.locator('.form-shell')).toHaveAttribute('data-state', 'create'); await expect(page.getByLabel(COPY.urlLabel)).toBeFocused(); }); diff --git a/frontend/src/__tests__/App.test.tsx b/frontend/src/__tests__/App.test.tsx index ac11a4dd..0bc6f800 100644 --- a/frontend/src/__tests__/App.test.tsx +++ b/frontend/src/__tests__/App.test.tsx @@ -148,7 +148,7 @@ describe('App', () => { fireEvent.click(screen.getByRole('button', { name: COPY.createFeed })); await waitFor(() => { - expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument(); + expect(screen.getByText(COPY.createFailedRetryTitle)).toBeInTheDocument(); }); } @@ -611,7 +611,7 @@ describe('App', () => { render(); expect(document.querySelector('.form-shell')).toHaveAttribute('data-state', 'error'); - expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument(); + expect(screen.getByText(COPY.createFailedRetryTitle)).toBeInTheDocument(); expect(screen.getByText('Access denied')).toBeInTheDocument(); }); @@ -625,7 +625,7 @@ describe('App', () => { retryable: false, nextAction: 'correct_input', retryAction: 'none', - message: 'This website blocked automated access.', + message: 'This site blocked automated access. Try another URL or site.', }, createFeed: mockCreateFeed, clearError: mockClearCreationError, @@ -635,7 +635,11 @@ describe('App', () => { render(); - expect(screen.getByText('This website blocked automated access.')).toBeInTheDocument(); + expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument(); + expect( + screen.getByText('This site blocked automated access. Try another URL or site.') + ).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: COPY.tryAgain })).not.toBeInTheDocument(); }); it('shows instance metadata failure as a banner without create-error chrome', () => { @@ -1046,6 +1050,8 @@ describe('App', () => { await screen.findByText( 'Could not extract feed items. Try a more specific listing URL or explicit selectors.' ); + expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument(); + expect(screen.queryByText(COPY.createFailedRetryTitle)).not.toBeInTheDocument(); expect(screen.queryByRole('heading', { name: COPY.tokenTitle })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: COPY.tryAgain })).not.toBeInTheDocument(); expect(mockClearToken).not.toHaveBeenCalled(); @@ -1076,7 +1082,7 @@ describe('App', () => { expect(location.hash).toMatch(/^#\/create/); }); expect(document.querySelector('dialog')).toBeNull(); - expect(screen.getByText(COPY.createFailedTitle)).toBeInTheDocument(); + expect(screen.getByText(COPY.createFailedRetryTitle)).toBeInTheDocument(); expect(screen.getByText('Upstream failed')).toBeInTheDocument(); }); diff --git a/frontend/src/__tests__/ResultDisplay.test.tsx b/frontend/src/__tests__/ResultDisplay.test.tsx index c34ebd34..810e6692 100644 --- a/frontend/src/__tests__/ResultDisplay.test.tsx +++ b/frontend/src/__tests__/ResultDisplay.test.tsx @@ -184,7 +184,7 @@ describe('ResultDisplay', () => { warnings: [ { code: 'PREVIEW_HTTP_422', - message: 'This website blocked automated access.', + message: 'This site blocked automated access. Try another URL or site.', retryable: false, nextAction: 'wait', }, @@ -195,7 +195,9 @@ describe('ResultDisplay', () => { /> ); - expect(screen.getByText('This website blocked automated access.')).toBeInTheDocument(); + expect( + screen.getByText('This site blocked automated access. Try another URL or site.') + ).toBeInTheDocument(); expect(screen.queryByRole('button', { name: COPY.checkAgain })).not.toBeInTheDocument(); }); diff --git a/frontend/src/__tests__/previewHydration.test.ts b/frontend/src/__tests__/previewHydration.test.ts index b88bf494..783086a5 100644 --- a/frontend/src/__tests__/previewHydration.test.ts +++ b/frontend/src/__tests__/previewHydration.test.ts @@ -80,7 +80,7 @@ describe('previewHydration', () => { it('passes short text/plain HTTP error bodies through as warning.message', async () => { fetchMock.mockResolvedValueOnce( - new Response('This website blocked automated access.', { + new Response('This site blocked automated access. Try another URL or site.', { status: 422, headers: { 'Content-Type': 'text/plain; charset=utf-8' }, }) @@ -91,7 +91,7 @@ describe('previewHydration', () => { warnings: [ { code: 'PREVIEW_HTTP_422', - message: 'This website blocked automated access.', + message: 'This site blocked automated access. Try another URL or site.', retryable: false, nextAction: 'wait', }, diff --git a/frontend/src/components/AppPanels.tsx b/frontend/src/components/AppPanels.tsx index 0992b0e5..b41888c8 100644 --- a/frontend/src/components/AppPanels.tsx +++ b/frontend/src/components/AppPanels.tsx @@ -324,7 +324,7 @@ function ActionFeedback({ diff --git a/frontend/src/journey/copy.ts b/frontend/src/journey/copy.ts index 77c47e60..ee84f075 100644 --- a/frontend/src/journey/copy.ts +++ b/frontend/src/journey/copy.ts @@ -10,7 +10,10 @@ export const COPY = { tokenHint: 'Required by this instance.', tokenPlaceholder: 'Paste access token', tokenRejected: 'Access token was rejected. Paste a valid token to continue.', - createFailedTitle: "Couldn't create feed yet", + /** Non-retryable / correct_input create failures (blocked, empty extract, …). */ + createFailedTitle: "Couldn't create feed", + /** Retryable create failures where Try again remains honest. */ + createFailedRetryTitle: "Couldn't create feed yet", tryAgain: 'Try again', checkAgain: 'Check again', previewItemCount: (count: number) => `${count} items`, diff --git a/spec/html2rss/web/error_classifier_spec.rb b/spec/html2rss/web/error_classifier_spec.rb index 9c1a4988..1ad2629d 100644 --- a/spec/html2rss/web/error_classifier_spec.rb +++ b/spec/html2rss/web/error_classifier_spec.rb @@ -67,7 +67,9 @@ def initialize(attempts:) end it 'uses one human sentence for classified user decisions', :aggregate_failures do - expect(described_class::BLOCKED_SURFACE.message).to eq('This website blocked automated access.') + expect(described_class::BLOCKED_SURFACE.message).to eq( + 'This site blocked automated access. Try another URL or site.' + ) expect(described_class::SCRAPER_UNAVAILABLE.message).to eq('Feed fetching is temporarily unavailable.') expect(described_class::SERVICE_UNAVAILABLE.message).to eq( 'The server is too busy or the request timed out.' From da52683427894faa9f4db610a8b404bde63c9150 Mon Sep 17 00:00:00 2001 From: Gil Desmarais Date: Fri, 28 Aug 2026 23:47:04 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(web):=20keep=20retry=20chrome=20after?= =?UTF-8?q?=20token=E2=86=92create=20failures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clearResult was wiping creationError, and remount/auto-submit raced away Decision metadata so retryable failures showed the non-retry title. --- frontend/src/__tests__/App.test.tsx | 23 +++++++++++--- frontend/src/__tests__/useFeedFlow.test.ts | 35 +++++++++++++++++----- frontend/src/feed/useFeedCreation.ts | 6 +++- frontend/src/feed/useFeedFlow.ts | 16 ++++++++-- 4 files changed, 65 insertions(+), 15 deletions(-) diff --git a/frontend/src/__tests__/App.test.tsx b/frontend/src/__tests__/App.test.tsx index 0bc6f800..424b027e 100644 --- a/frontend/src/__tests__/App.test.tsx +++ b/frontend/src/__tests__/App.test.tsx @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { render, screen, fireEvent, waitFor } from '@testing-library/preact'; +import type { FeedCreationError } from '../api/contracts'; import { App } from '../components/App'; import { COPY } from '../journey/copy'; @@ -49,6 +50,7 @@ const mockCreatedFeedResult = { async function expectCreateRemountedWithoutErrorChrome() { await waitFor(() => { expect(screen.queryByText(COPY.createFailedTitle)).not.toBeInTheDocument(); + expect(screen.queryByText(COPY.createFailedRetryTitle)).not.toBeInTheDocument(); }); expect(document.querySelector('.form-shell')).toHaveAttribute('data-state', 'create'); await waitFor(() => { @@ -63,12 +65,18 @@ describe('App', () => { const mockClearCreationError = vi.fn(); const mockClearResult = vi.fn(); const mockRetryPreviewFetch = vi.fn(); + /** Mirrors useFeedCreation: reject sets error; clearError clears it (App mock was static). */ + let creationHookError: FeedCreationError | undefined; beforeEach(() => { vi.clearAllMocks(); + creationHookError = undefined; history.replaceState({}, '', 'http://localhost:3000/#/create'); localStorage.clear(); mockCreateFeed.mockResolvedValue(mockCreatedFeedResult); + mockClearCreationError.mockImplementation(() => { + creationHookError = undefined; + }); mockUseCatalogEntries.mockReturnValue([]); mockUseAccessToken.mockReturnValue({ @@ -99,15 +107,22 @@ describe('App', () => { error: undefined, }); - mockUseFeedCreation.mockReturnValue({ + mockUseFeedCreation.mockImplementation(() => ({ isCreating: false, result: undefined, - error: undefined, - createFeed: mockCreateFeed, + error: creationHookError, + createFeed: async (url: string, token: string) => { + try { + return await mockCreateFeed(url, token); + } catch (error) { + creationHookError = error as FeedCreationError; + throw error; + } + }, clearError: mockClearCreationError, clearResult: mockClearResult, retryPreviewFetch: mockRetryPreviewFetch, - }); + })); }); const creationFailure = { diff --git a/frontend/src/__tests__/useFeedFlow.test.ts b/frontend/src/__tests__/useFeedFlow.test.ts index c666f4ac..b9e35a78 100644 --- a/frontend/src/__tests__/useFeedFlow.test.ts +++ b/frontend/src/__tests__/useFeedFlow.test.ts @@ -187,13 +187,19 @@ describe('useFeedFlow', () => { it('returns non-auth creation failures from token onto create', async () => { await stubFeedCreationFailure(); - const { result } = renderHook(() => - useFeedFlow( - feedFlowProperties({ - mayCreate: () => 'proceed', - route: { kind: 'token', prefillUrl: 'https://example.com/private-articles' }, - }) - ) + const { result, rerender } = renderHook( + ({ route }) => + useFeedFlow( + feedFlowProperties({ + mayCreate: () => 'proceed', + route, + }) + ), + { + initialProps: { + route: { kind: 'token' as const, prefillUrl: 'https://example.com/private-articles' }, + }, + } ); act(() => { @@ -213,6 +219,21 @@ describe('useFeedFlow', () => { }); expect(result.current.tokenError).toBe(''); expect(result.current.feedFieldErrors.form).toBe('Upstream failed'); + expect(result.current.creationError).toMatchObject({ + message: 'Upstream failed', + nextAction: 'retry', + }); + + // Simulate router applying token→create; remount must keep Decision error for retry chrome. + rerender({ + route: { kind: 'create', prefillUrl: 'https://example.com/private-articles' }, + }); + + expect(result.current.feedFieldErrors.form).toBe('Upstream failed'); + expect(result.current.creationError).toMatchObject({ + message: 'Upstream failed', + nextAction: 'retry', + }); }); it('recovers unmatched result routes onto remounted create without prefillUrl', async () => { diff --git a/frontend/src/feed/useFeedCreation.ts b/frontend/src/feed/useFeedCreation.ts index 192df98d..360ac7f5 100644 --- a/frontend/src/feed/useFeedCreation.ts +++ b/frontend/src/feed/useFeedCreation.ts @@ -62,7 +62,11 @@ export function useFeedCreation() { globalThis.document?.body?.scrollIntoView({ behavior: 'smooth', block: 'start' }); requestIdReference.current += 1; cancelPreview(); - setState({ isCreating: false }); + setState((previous) => ({ + ...previous, + isCreating: false, + result: undefined, + })); }; const clearError = () => { diff --git a/frontend/src/feed/useFeedFlow.ts b/frontend/src/feed/useFeedFlow.ts index 244cdc5e..3f0d3bb1 100644 --- a/frontend/src/feed/useFeedFlow.ts +++ b/frontend/src/feed/useFeedFlow.ts @@ -141,6 +141,8 @@ export function useFeedFlow({ setFeedFieldErrors({ ...EMPTY_FEED_ERRORS, form: failure.message }); if (route.kind === 'token') { + // Prevent create-entry auto-submit from clearing Decision error before remount runs. + hasAutoSubmittedReference.current = true; navigate({ kind: 'create', prefillUrl: normalizedUrl }); } return false; @@ -212,14 +214,22 @@ export function useFeedFlow({ const isSameKindCreateEntry = previousKind === 'create' && previousCreateEntryKey !== createEntryKey; if (!didKindChangeToCreate && !isSameKindCreateEntry) return; - clearError(); clearResult(); setTokenError(''); setTokenDraft(''); - if (isSameKindCreateEntry) setFeedFieldErrors(EMPTY_FEED_ERRORS); + if (isSameKindCreateEntry) { + setFeedFieldErrors(EMPTY_FEED_ERRORS); + clearError(); + } else if (feedFieldErrors.form) { + // token→create non-auth failure: keep Decision error for retry chrome; do not auto-retry. + hasAutoSubmittedReference.current = true; + } else { + // Kind change onto create without a projected form failure (e.g. result recovery). + clearError(); + } if (!route.prefillUrl) autoSubmitUrlReference.current = undefined; setFocusCreateComposerKey((current) => current + 1); - }, [createEntryKey, route]); + }, [clearError, clearResult, createEntryKey, feedFieldErrors.form, route]); const viewModel = decideJourney({ creationError,