Review feedback on the filed request: - R1: gui/CMakeLists.txt is on main now, so root's find_package(Qt6 ... Svg REQUIRED) is live — a missing SVG dev package is a hard configure failure for the whole project, not a skipped guard. Added the reason CI hasn't caught it: ubuntu-latest is 24.04 (where libqt6svg6-dev likely resolves), the project targets 26.04 (where it does not). Wrong name + runner/target release mismatch = the class of bug PKG/QA owns is currently unobservable in CI. That's the argument for R2, folded in. - R3: corrected — CI does build the GUI and runs its three ctests under the ci preset. What has no home is the non-unit-test DoD: 10k-row 60fps, flat RSS over a 10-minute run, and mockd --slow/--flaky/--drop-connection recovery. Asked for those specifically. - gui/docs/ext-requests-m1.md: CLAUDE.md §3 says the no-download-logic rule applies to extension/ too; GUI made its half an executable ctest, EXT's half is still prose. Suggested the ESLint equivalent for the existing extension-lint job. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_016Ne28kx4VreeBWZv82Nksd
34 lines
1.5 KiB
Markdown
34 lines
1.5 KiB
Markdown
# GUI → EXT requests (M1)
|
|
|
|
Filed by lane GUI. `extension/` is EXT's; this is a suggestion, not a change.
|
|
|
|
## An executable version of the "no download logic in extension/" rule
|
|
|
|
CLAUDE.md §3:
|
|
|
|
> A grep for `curl|pwrite|sqlite` in `gui/` must come back empty. **Same for download
|
|
> logic in `extension/`.**
|
|
|
|
GUI turned its half of that sentence into a gate: `gui/tests/no_download_logic.cmake`, run
|
|
as the `gui_no_download_logic` ctest, greps `gui/src` for `curl_*` / `pwrite` / `sqlite` /
|
|
`QSqlDatabase` / `QNetworkAccessManager` and fails the build on a hit. It was verified by
|
|
dropping a `curl_`/`sqlite3_` file in and watching it go red.
|
|
|
|
The `extension/` half is still just prose. The extension is the more tempting place for
|
|
download logic to creep in — a `fetch()` to grab bytes "just this once", a stream reader,
|
|
a `Range` header assembled client-side — and none of that would fail any current check.
|
|
|
|
Suggested equivalent for EXT (adjust tokens to the TS/WebExtension surface):
|
|
|
|
* `fetch(` / `XMLHttpRequest` / `new Request(` used for anything but talking to the
|
|
native host,
|
|
* `Range:` / `Content-Range` header construction,
|
|
* `ReadableStream` / `.getReader()` over response bodies,
|
|
* `IndexedDB` / `chrome.downloads` / `browser.downloads` used to move bytes rather than
|
|
hand off.
|
|
|
|
Cheapest form is an ESLint `no-restricted-syntax` / `no-restricted-globals` rule in the
|
|
`extension-lint` CI job (already exists in `ci.yml`), so it runs per-PR with no new
|
|
infrastructure. The point isn't the exact token list — it's that the rule fails a build
|
|
instead of a review.
|