fix: bot-review robustness fixes (TTS sync, updater, nightly, a11y) (#4659)
Cherry-picked and re-verified the applicable subset of julianshen/readest@fa1b74a0 (its "address PR #15 bot reviews" commit). The fork-only AI-annotation-tool change was dropped — readest has no 'ai' toolbar tool. Each logic fix is covered by a failing-first test. - TTS position sequence is now an app-wide monotonic counter, so a fresh TTSController (constructed per `tts-speak`) isn't dropped by consumers holding `lastSequenceSeen` from a prior session. - share.ts only swallows AbortError (user cancel); other failures — e.g. NotAllowedError when a quick action fires without a user gesture — fall back to the clipboard so the text still reaches the user. - document.isTxt tolerates MIME params (text/plain;charset=utf-8), uppercase extensions (BOOK.TXT), and a nameless Blob, so a TXT can't slip onto the non-text path and yield a null book. - updater getNightlyPlatformKey matches x86_64/aarch64 explicitly; a 32-bit or otherwise unknown arch yields no nightly instead of mis-routing to aarch64. - UpdaterWindow downloadWithProgress resolves on tauriDownload completion even when Content-Length is absent (no more hang on portable/AppImage/Android). - nightly_update.rs uses async tokio::fs::read in the async command. - nightly.yml: serialize runs via a concurrency group (no cancel) and persist-credentials:false on checkouts. - edge TTS route only emits the word-boundary header when it fits under ~8KB; oversized values get dropped by proxies, and the client falls back to []. - RSVPOverlay drops the contradictory aria-disabled on the functional rate button (it opens the pace picker). - nightly verify harness handles artifact stream errors instead of crashing. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -16,6 +16,12 @@ on:
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
# Serialize runs so an older run can't publish nightly/latest.json after a newer
|
||||
# one (no cancel — let an in-flight build finish rather than drop artifacts).
|
||||
concurrency:
|
||||
group: nightly-readest
|
||||
cancel-in-progress: false
|
||||
|
||||
jobs:
|
||||
compute-version:
|
||||
runs-on: ubuntu-latest
|
||||
@@ -25,6 +31,7 @@ jobs:
|
||||
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
|
||||
with:
|
||||
ref: main
|
||||
persist-credentials: false
|
||||
- id: v
|
||||
run: |
|
||||
BASE=$(node -p "require('./apps/readest-app/package.json').version")
|
||||
@@ -70,6 +77,7 @@ jobs:
|
||||
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
|
||||
with:
|
||||
ref: main
|
||||
persist-credentials: false
|
||||
|
||||
- name: initialize git submodules
|
||||
run: git submodule update --init --recursive
|
||||
|
||||
@@ -89,9 +89,17 @@ const handleRequest = (req, res) => {
|
||||
return json(res, buildStableManifest(false));
|
||||
case '/releases/latest-surpass.json':
|
||||
return json(res, buildStableManifest(true));
|
||||
case '/artifacts/test.bin':
|
||||
case '/artifacts/test.bin': {
|
||||
const stream = fs.createReadStream(ARTIFACT);
|
||||
// Handle a missing/unreadable artifact instead of crashing the harness on
|
||||
// an unhandled stream 'error'.
|
||||
stream.on('error', () => {
|
||||
if (!res.headersSent) res.writeHead(500);
|
||||
res.end('artifact error');
|
||||
});
|
||||
res.writeHead(200, { 'content-type': 'application/octet-stream' });
|
||||
return fs.createReadStream(ARTIFACT).pipe(res);
|
||||
return stream.pipe(res);
|
||||
}
|
||||
default:
|
||||
res.writeHead(404);
|
||||
return res.end('not found');
|
||||
|
||||
@@ -49,7 +49,7 @@ fn base64_to_string(s: &str) -> Option<String> {
|
||||
/// nightly artifact accepted here is also accepted by Tauri's installer.
|
||||
#[tauri::command]
|
||||
pub async fn verify_update_signature(path: String, signature: String, pub_key: String) -> bool {
|
||||
let Ok(data) = std::fs::read(&path) else {
|
||||
let Ok(data) = tokio::fs::read(&path).await else {
|
||||
return false;
|
||||
};
|
||||
verify_signature_impl(&data, &signature, &pub_key)
|
||||
|
||||
@@ -426,6 +426,18 @@ describe('getNightlyPlatformKey', () => {
|
||||
test('macos', () => {
|
||||
expect(getNightlyPlatformKey('macos', 'aarch64', false, false)).toBe('darwin-aarch64');
|
||||
});
|
||||
test('windows/linux aarch64 still map to their arm keys', () => {
|
||||
expect(getNightlyPlatformKey('windows', 'aarch64', false, false)).toBe('windows-aarch64');
|
||||
expect(getNightlyPlatformKey('windows', 'aarch64', true, false)).toBe(
|
||||
'windows-aarch64-portable',
|
||||
);
|
||||
expect(getNightlyPlatformKey('linux', 'aarch64', false, true)).toBe('linux-aarch64-appimage');
|
||||
});
|
||||
test('an unknown / 32-bit arch yields no nightly (no aarch64 mis-route)', () => {
|
||||
expect(getNightlyPlatformKey('windows', 'i686', false, false)).toBeNull();
|
||||
expect(getNightlyPlatformKey('windows', 'i686', true, false)).toBeNull();
|
||||
expect(getNightlyPlatformKey('linux', 'i686', false, true)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveNightlyUpdate', () => {
|
||||
|
||||
@@ -70,4 +70,27 @@ describe('DocumentLoader.open', () => {
|
||||
expect(result.book).toBeTruthy();
|
||||
expect(result.format).toBe('EPUB');
|
||||
}, 15000);
|
||||
|
||||
const txtBody = ['Chapter 1', '', 'Hello world.'].join('\n');
|
||||
|
||||
it('opens a .txt whose MIME carries a charset param and whose name lacks the ext', async () => {
|
||||
// Android can hand over a content:// URI with no .txt extension and a
|
||||
// parameterised MIME (text/plain;charset=utf-8). Strict `=== 'text/plain'`
|
||||
// plus a name-only ext check would both miss and yield a null book.
|
||||
const file = new File([txtBody], 'content-12345', { type: 'text/plain;charset=utf-8' });
|
||||
|
||||
const result = await new DocumentLoader(file).open();
|
||||
|
||||
expect(result.book).toBeTruthy();
|
||||
expect(result.format).toBe('EPUB');
|
||||
}, 15000);
|
||||
|
||||
it('opens a .txt with an uppercase extension and a generic MIME', async () => {
|
||||
const file = new File([txtBody], 'MY-BOOK.TXT', { type: 'application/octet-stream' });
|
||||
|
||||
const result = await new DocumentLoader(file).open();
|
||||
|
||||
expect(result.book).toBeTruthy();
|
||||
expect(result.format).toBe('EPUB');
|
||||
}, 15000);
|
||||
});
|
||||
|
||||
@@ -825,6 +825,38 @@ describe('TTSController', () => {
|
||||
expect(sequences[i]!).toBeGreaterThan(sequences[i - 1]!);
|
||||
}
|
||||
});
|
||||
|
||||
test('a fresh controller continues the sequence instead of restarting', async () => {
|
||||
vi.mocked(mockView.getCFI).mockReturnValue('cfi-x');
|
||||
|
||||
// Emit one sentence position from a controller and return its sequence.
|
||||
const emitOnce = async (c: TTSController) => {
|
||||
await c.initViewTTS(0);
|
||||
mockView.tts = {
|
||||
setMark: vi.fn().mockReturnValue(makeSentenceRange()),
|
||||
getLastRange: vi.fn().mockImplementation(() => makeSentenceRange()),
|
||||
} as unknown as FoliateView['tts'];
|
||||
let seq = -1;
|
||||
const handler = (e: Event) => {
|
||||
seq = (e as CustomEvent).detail.sequence;
|
||||
};
|
||||
c.addEventListener('tts-position', handler);
|
||||
c.dispatchSpeakMark({ offset: 0, name: '0', text: 'hello', language: 'en' });
|
||||
c.removeEventListener('tts-position', handler);
|
||||
return seq;
|
||||
};
|
||||
|
||||
const firstSeq = await emitOnce(controller);
|
||||
// A new `tts-speak` builds a fresh TTSController (see useTTSControl). A
|
||||
// per-instance counter would restart, so the new session's first sequence
|
||||
// would be <= the previous session's and a consumer holding
|
||||
// `lastSequenceSeen` would drop it. A module-level counter keeps the
|
||||
// sequence strictly increasing across sessions.
|
||||
const controller2 = new TTSController(mockAppService, mockView, false);
|
||||
const secondSeq = await emitOnce(controller2);
|
||||
|
||||
expect(secondSeq).toBeGreaterThan(firstSeq);
|
||||
});
|
||||
});
|
||||
|
||||
describe('redispatchPosition', () => {
|
||||
|
||||
@@ -62,13 +62,25 @@ describe('shareSelectedText', () => {
|
||||
expect(writeClipboardMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('swallows navigator.share rejection (user dismissed) without clipboard fallback', async () => {
|
||||
const navShare = vi.fn().mockRejectedValue(new Error('AbortError'));
|
||||
test('swallows an AbortError (user dismissed) without clipboard fallback', async () => {
|
||||
const abortErr = new Error('user dismissed');
|
||||
abortErr.name = 'AbortError';
|
||||
const navShare = vi.fn().mockRejectedValue(abortErr);
|
||||
globalThis.navigator.share = navShare;
|
||||
await expect(shareSelectedText('hello', undefined, null)).resolves.toBeUndefined();
|
||||
expect(writeClipboardMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('falls back to clipboard when navigator.share fails for a non-Abort reason', async () => {
|
||||
// e.g. NotAllowedError when a quick action fires without a user gesture.
|
||||
const notAllowed = new Error('permission denied');
|
||||
notAllowed.name = 'NotAllowedError';
|
||||
const navShare = vi.fn().mockRejectedValue(notAllowed);
|
||||
globalThis.navigator.share = navShare;
|
||||
await shareSelectedText('hello', undefined, null);
|
||||
expect(writeClipboardMock).toHaveBeenCalledWith('hello');
|
||||
});
|
||||
|
||||
test('falls back to clipboard when no share method exists', async () => {
|
||||
await shareSelectedText('hello', undefined, null);
|
||||
expect(shareTextMock).not.toHaveBeenCalled();
|
||||
|
||||
@@ -77,14 +77,19 @@ export async function POST(request: NextRequest) {
|
||||
const { response, boundaries } = await tts.createWithBoundaries(payload);
|
||||
const arrayBuffer = await response.arrayBuffer();
|
||||
|
||||
return new NextResponse(arrayBuffer, {
|
||||
status: 200,
|
||||
headers: {
|
||||
'Content-Type': 'audio/mpeg',
|
||||
'Content-Length': arrayBuffer.byteLength.toString(),
|
||||
[WORD_BOUNDARIES_HEADER]: serializeWordBoundaries(boundaries),
|
||||
},
|
||||
});
|
||||
const headers: Record<string, string> = {
|
||||
'Content-Type': 'audio/mpeg',
|
||||
'Content-Length': arrayBuffer.byteLength.toString(),
|
||||
};
|
||||
// Only emit the word-boundary header when it fits well under common
|
||||
// header-size caps (~8KB). Oversized values can be dropped by proxies and
|
||||
// break delivery; the client falls back to [] when the header is absent.
|
||||
const serializedBoundaries = serializeWordBoundaries(boundaries);
|
||||
if (serializedBoundaries.length <= 8192) {
|
||||
headers[WORD_BOUNDARIES_HEADER] = serializedBoundaries;
|
||||
}
|
||||
|
||||
return new NextResponse(arrayBuffer, { status: 200, headers });
|
||||
} catch (error) {
|
||||
console.error('Edge TTS API error:', error);
|
||||
return NextResponse.json(
|
||||
|
||||
@@ -760,15 +760,14 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
|
||||
{/* WPM selector — while RSVP follows TTS the timer no longer paces, so it
|
||||
becomes an "Audio pace" affordance that opens a TTS rate picker
|
||||
instead (decision 6). aria-disabled (not hard-disabled) keeps it
|
||||
focusable as a hint; the lock glyph + border reads in e-ink without
|
||||
relying on opacity. */}
|
||||
instead (decision 6). It stays a real, enabled button (it opens the
|
||||
picker), so no aria-disabled; the lock glyph + border reads in e-ink
|
||||
without relying on opacity. */}
|
||||
<div className='relative shrink-0'>
|
||||
{ttsDriven ? (
|
||||
<button
|
||||
className='eink-bordered flex items-center gap-1.5 rounded-full border border-gray-500/20 bg-gray-500/10 px-3 py-1.5 text-sm transition-colors hover:bg-gray-500/20'
|
||||
onClick={() => setShowRateDropdown(!showRateDropdown)}
|
||||
aria-disabled='true'
|
||||
aria-label={_('Audio pace')}
|
||||
title={_('Speed follows audio')}
|
||||
>
|
||||
|
||||
@@ -186,40 +186,33 @@ export const UpdaterContent = ({
|
||||
} as GenericUpdate);
|
||||
}
|
||||
};
|
||||
const downloadWithProgress = (
|
||||
const downloadWithProgress = async (
|
||||
downloadUrl: string,
|
||||
filePath: string,
|
||||
onEvent?: (progress: DownloadEvent) => void,
|
||||
): Promise<void> => {
|
||||
return new Promise<void>(async (resolve, reject) => {
|
||||
let downloaded = 0;
|
||||
let total = 0;
|
||||
await tauriDownload(downloadUrl, filePath, (progress) => {
|
||||
if (!onEvent) return;
|
||||
if (!total && progress.total) {
|
||||
total = progress.total;
|
||||
onEvent({
|
||||
event: 'Started',
|
||||
data: { contentLength: total },
|
||||
});
|
||||
} else if (downloaded > 0 && progress.progress === progress.total) {
|
||||
console.log('File downloaded to', filePath);
|
||||
onEvent?.({ event: 'Finished' });
|
||||
setTimeout(() => {
|
||||
resolve();
|
||||
}, 1000);
|
||||
}
|
||||
|
||||
onEvent({
|
||||
event: 'Progress',
|
||||
data: { chunkLength: progress.progress - downloaded },
|
||||
});
|
||||
downloaded = progress.progress;
|
||||
}).catch((error) => {
|
||||
console.error('Download failed:', error);
|
||||
reject(error);
|
||||
});
|
||||
let downloaded = 0;
|
||||
let total = 0;
|
||||
let finished = false;
|
||||
// Resolve when tauriDownload itself completes — NOT only when a progress
|
||||
// tick reports progress === total. Servers that omit Content-Length leave
|
||||
// total at 0, so that tick never fires and the await would hang forever
|
||||
// after the file is fully written (nightly portable/AppImage/Android).
|
||||
await tauriDownload(downloadUrl, filePath, (progress) => {
|
||||
if (!onEvent) return;
|
||||
if (!total && progress.total) {
|
||||
total = progress.total;
|
||||
onEvent({ event: 'Started', data: { contentLength: total } });
|
||||
}
|
||||
onEvent({ event: 'Progress', data: { chunkLength: progress.progress - downloaded } });
|
||||
downloaded = progress.progress;
|
||||
if (progress.total && progress.progress === progress.total && !finished) {
|
||||
finished = true;
|
||||
onEvent({ event: 'Finished' });
|
||||
}
|
||||
});
|
||||
console.log('File downloaded to', filePath);
|
||||
if (onEvent && !finished) onEvent({ event: 'Finished' });
|
||||
};
|
||||
const checkWindowsPortableUpdate = async () => {
|
||||
if (!appService) return;
|
||||
|
||||
@@ -63,18 +63,23 @@ export const getNightlyPlatformKey = (
|
||||
isPortable: boolean,
|
||||
isAppImage: boolean,
|
||||
): string | null => {
|
||||
const is64 = osArchVal === 'x86_64';
|
||||
if (osTypeVal === 'android')
|
||||
return osArchVal === 'aarch64' ? 'android-arm64' : 'android-universal';
|
||||
if (osTypeVal === 'macos') return osArchVal === 'aarch64' ? 'darwin-aarch64' : 'darwin-x86_64';
|
||||
// Match the arch explicitly so a 32-bit (or otherwise unknown) arch yields no
|
||||
// nightly rather than mis-routing to aarch64.
|
||||
if (osTypeVal === 'windows') {
|
||||
if (isPortable) return is64 ? 'windows-x86_64-portable' : 'windows-aarch64-portable';
|
||||
return is64 ? 'windows-x86_64' : 'windows-aarch64';
|
||||
if (osArchVal === 'x86_64') return isPortable ? 'windows-x86_64-portable' : 'windows-x86_64';
|
||||
if (osArchVal === 'aarch64') return isPortable ? 'windows-aarch64-portable' : 'windows-aarch64';
|
||||
return null;
|
||||
}
|
||||
if (osTypeVal === 'linux') {
|
||||
// Nightly Linux is AppImage-only; a deb/rpm install has no nightly
|
||||
// artifact, so it cleanly gets no nightly rather than mis-routing.
|
||||
if (isAppImage) return is64 ? 'linux-x86_64-appimage' : 'linux-aarch64-appimage';
|
||||
if (isAppImage) {
|
||||
if (osArchVal === 'x86_64') return 'linux-x86_64-appimage';
|
||||
if (osArchVal === 'aarch64') return 'linux-aarch64-appimage';
|
||||
}
|
||||
return null;
|
||||
}
|
||||
return null;
|
||||
|
||||
@@ -328,7 +328,13 @@ export class DocumentLoader {
|
||||
}
|
||||
|
||||
private isTxt(): boolean {
|
||||
return this.file.type === 'text/plain' || this.file.name.endsWith(`.${EXTS.TXT}`);
|
||||
// Tolerate MIME params (text/plain;charset=utf-8), uppercase extensions
|
||||
// (BOOK.TXT), and a nameless Blob — otherwise a TXT can slip onto the
|
||||
// non-text path and yield a null book.
|
||||
return (
|
||||
this.file.type.startsWith('text/plain') ||
|
||||
(this.file.name?.toLowerCase().endsWith(`.${EXTS.TXT}`) ?? false)
|
||||
);
|
||||
}
|
||||
|
||||
public async open(): Promise<{ book: BookDoc; format: BookFormat }> {
|
||||
|
||||
@@ -17,6 +17,14 @@ import {
|
||||
TTSWordOffset,
|
||||
} from './wordHighlight';
|
||||
|
||||
// App-wide monotonic sequence for 'tts-position' events. A fresh TTSController
|
||||
// is constructed per `tts-speak`, so a per-instance counter would restart at 0
|
||||
// and consumers (paragraph mode, RSVP) holding `lastSequenceSeen` from a prior
|
||||
// session would drop the new session's early positions until they exceeded the
|
||||
// old count. A module-level counter keeps the sequence strictly increasing
|
||||
// across sessions.
|
||||
let ttsPositionSequence = 0;
|
||||
|
||||
type TTSState =
|
||||
| 'stopped'
|
||||
| 'playing'
|
||||
@@ -41,10 +49,6 @@ export class TTSController extends EventTarget {
|
||||
|
||||
#ttsSectionIndex: number = -1;
|
||||
|
||||
// Monotonic counter for the canonical 'tts-position' event so downstream
|
||||
// consumers (paragraph mode, RSVP) can drop out-of-order positions.
|
||||
#positionSequence: number = 0;
|
||||
|
||||
// Word-level highlight state for the currently spoken chunk. Armed by a
|
||||
// successful dispatchSpeakMark, populated by prepareSpeakWords when a TTS
|
||||
// client has word-boundary metadata for the chunk.
|
||||
@@ -614,7 +618,7 @@ export class TTSController extends EventTarget {
|
||||
cfi,
|
||||
kind,
|
||||
sectionIndex: this.#ttsSectionIndex,
|
||||
sequence: ++this.#positionSequence,
|
||||
sequence: ++ttsPositionSequence,
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -99,10 +99,13 @@ export const shareSelectedText = async (
|
||||
if (typeof navigator !== 'undefined' && typeof navigator.share === 'function') {
|
||||
try {
|
||||
await navigator.share({ text });
|
||||
} catch {
|
||||
// User dismissed or share-time error; respect the choice.
|
||||
return;
|
||||
} catch (err) {
|
||||
// Only respect a user cancel (AbortError). Other failures — e.g.
|
||||
// NotAllowedError when a quick action fires without a user gesture —
|
||||
// fall through to the clipboard so the user still gets the text.
|
||||
if (err instanceof Error && err.name === 'AbortError') return;
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
await writeTextToClipboard(text);
|
||||
|
||||
Reference in New Issue
Block a user