From 9a8175903b6dd409d715d46b68ce4835bb7a699c Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 10 Jun 2026 20:20:36 +0200 Subject: [PATCH] pql: retry transient db-busy so sidebar panes don't stick (T-350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pql-backed sidebar panes fetch once on first build. If that fetch fired too early — the planning DB still settling at startup, or a db-busy SQLite lock under concurrent pql writes (pql exits 69) — the pane showed "pql … failed" and stayed there until a manual refresh re-fired it. Retry transient failures at the single chokepoint, PqlClient._run: on a busy/locked signal (exit 69, or stderr mentioning database is locked / busy) retry a few times with short backoff before throwing. Genuine errors aren't busy, so they still surface immediately. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 1 + .pql/changelog/tickets/2026-06.sql | 6 ++ CHANGELOG.md | 8 +++ lib/src/pql/client.dart | 78 +++++++++++++++-------- test/pql/client_test.dart | 39 ++++++++++++ 5 files changed, 106 insertions(+), 26 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index f503b756..5a6959bf 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -3510,3 +3510,4 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB4XCM5KBXDDSCWJ37GPYG3R', 'status', 'backlog', 'done', NULL, '2026-06-10 16:57:07', '2026-06-10 16:57:07', '2026-06-10 16:57:07', NULL, '1ce291d54831d3ede687384037c379ff', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB50YE6S6YWNP2ZSFWES9B2W', 'status', 'backlog', 'done', NULL, '2026-06-10 17:13:53', '2026-06-10 17:13:53', '2026-06-10 17:13:53', NULL, '87b069d3f25c91622c9720147d359360', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB58X0TFJ02YTMVPD0D9Q838', 'status', 'backlog', 'done', NULL, '2026-06-10 17:47:01', '2026-06-10 17:47:01', '2026-06-10 17:47:01', NULL, 'ba3d17ec4e668ade82b07d2bb848ab91', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB5CW7JPT6BR2RWMNYVCXJ50', 'status', 'backlog', 'done', NULL, '2026-06-10 18:20:28', '2026-06-10 18:20:28', '2026-06-10 18:20:28', NULL, '213109fcd67b4375ebbd3b59c4a1ed04', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index 2d25928d..ef43cc19 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -3213,3 +3213,9 @@ INSERT INTO tickets (record_id, type, parent_record_id, title, description, stat So a build host/container missing the Wayland client dev headers (very plausible in a Bazzite/immutable distrobox or toolbox) silently compiles out the decoration suppression — the rebuilt app ships with the compositor''s native title bar (double title bar on KDE Plasma Wayland). Reported live on Bazzite KDE. Fix: make wayland-client a hard build requirement — fail the CMake configure with a clear, actionable message (name the package: Fedora wayland-devel, Debian/Ubuntu libwayland-dev) instead of dropping the feature. Frameless chrome is a core guardrail; never ship without it. Also fix the stale ''xdg-decoration'' comment (the code uses the KDE server-decoration protocol, not xdg-decoration).', 'done', 'high', NULL, NULL, NULL, '2026-06-10 17:45:28', '2026-06-10 17:47:01', NULL, '6eb6120c0ef64a7f5ce78073b398143c', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB5CW7JPT6BR2RWMNYVCXJ50', 'bug', '06FB0TNQM5TWC00GW0P3X02HZW', 'pql sidebar panes stick on error when the first fetch fires too early', 'The pql-backed sidebar panes (tickets, decisions, pql, search) fetch once on first build via pql.* IPC. If that first fetch hits a TRANSIENT pql failure — the planning DB still settling at startup, or a db-busy SQLite lock under concurrent pql writes (pql 1.10 exits 69 ''db busy'', as seen in the serial test suite) — the pane shows ''pql ticket failed'' and stays there until a manual refresh / tab-switch re-fires the fetch. Reproduced live on 2.3.0: left pane ''pql ticket failed''; works on manual refresh. pql works fine in isolation, so it''s purely a too-early / transient timing issue with no retry. + +Fix: make pql invocations resilient to transient failures at the single chokepoint, PqlClient._run (lib/src/pql/client.dart) — on a busy/locked signal (exit 69, or stderr mentioning database is locked / busy), retry a small bounded number of times with short backoff before throwing PqlException. Keep genuine errors fast (don''t blanket-retry every non-zero). Fixes all pql panes at once.', 'backlog', 'high', NULL, NULL, NULL, '2026-06-10 18:02:50', '2026-06-10 18:02:50', NULL, 'd03c864f5e64284f3582e59556a4ab4f', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FB5CW7JPT6BR2RWMNYVCXJ50', 'bug', '06FB0TNQM5TWC00GW0P3X02HZW', 'pql sidebar panes stick on error when the first fetch fires too early', 'The pql-backed sidebar panes (tickets, decisions, pql, search) fetch once on first build via pql.* IPC. If that first fetch hits a TRANSIENT pql failure — the planning DB still settling at startup, or a db-busy SQLite lock under concurrent pql writes (pql 1.10 exits 69 ''db busy'', as seen in the serial test suite) — the pane shows ''pql ticket failed'' and stays there until a manual refresh / tab-switch re-fires the fetch. Reproduced live on 2.3.0: left pane ''pql ticket failed''; works on manual refresh. pql works fine in isolation, so it''s purely a too-early / transient timing issue with no retry. + +Fix: make pql invocations resilient to transient failures at the single chokepoint, PqlClient._run (lib/src/pql/client.dart) — on a busy/locked signal (exit 69, or stderr mentioning database is locked / busy), retry a small bounded number of times with short backoff before throwing PqlException. Keep genuine errors fast (don''t blanket-retry every non-zero). Fixes all pql panes at once.', 'done', 'high', NULL, NULL, NULL, '2026-06-10 18:02:50', '2026-06-10 18:20:28', NULL, 'd48c86ad83f3da7cd4f8c9308c852448', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index 8189146e..42137d8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,14 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ## [Unreleased] +### Fixed + +- **pql sidebar panes no longer stick on a transient startup error.** A + too-early or db-busy pql failure (the planning DB still settling, or a + SQLite lock under concurrent writes) is now retried a few times before + surfacing, instead of leaving the pane on "pql … failed" until a manual + refresh. (T-350) + ## [2.3.0] — 2026-06-10 ### Fixed diff --git a/lib/src/pql/client.dart b/lib/src/pql/client.dart index 05dba639..47e8eab7 100644 --- a/lib/src/pql/client.dart +++ b/lib/src/pql/client.dart @@ -162,33 +162,59 @@ class PqlClient { return const {}; } + /// pql's exit code for a locked / unavailable planning DB (EX_UNAVAILABLE) — + /// a transient SQLite-busy condition under concurrent access (T-350). + static const int _kBusyExitCode = 69; + static const int _kMaxAttempts = 4; + Future _run(List args) async { - final ProcessResult r; - try { - r = await Process.run( - toolchain.pql, - args, - workingDirectory: workDir.path, - ); - } on ProcessException catch (e) { - throw PqlException( - 'pql ${args.first}: ${e.message}', - exitCode: e.errorCode, - stderr: e.toString(), - ); + for (var attempt = 1; attempt <= _kMaxAttempts; attempt++) { + final ProcessResult r; + try { + r = await Process.run( + toolchain.pql, + args, + workingDirectory: workDir.path, + ); + } on ProcessException catch (e) { + throw PqlException( + 'pql ${args.first}: ${e.message}', + exitCode: e.errorCode, + stderr: e.toString(), + ); + } + final stderr = (r.stderr as String).trim(); + // pql 1.5+ returns exit 0 with an empty `[]` for zero matches, so any + // non-zero exit is a real error (older pql used exit 2 for empty). + if (r.exitCode != 0) { + // A transient db-busy / still-settling failure — a sidebar pane firing + // its one-shot fetch too early at startup, or contention from + // concurrent pql writes — would otherwise stick until a manual refresh. + // Retry a few times with short backoff first. Genuine errors aren't + // busy, so they still surface immediately. (T-350) + if (attempt < _kMaxAttempts && _isTransient(r.exitCode, stderr)) { + await Future.delayed(Duration(milliseconds: 100 * attempt)); + continue; + } + throw PqlException( + 'pql ${args.first} failed', + exitCode: r.exitCode, + stderr: stderr, + ); + } + final stdout = (r.stdout as String).trim(); + if (stdout.isEmpty) return null; + return jsonDecode(stdout); } - final stderr = (r.stderr as String).trim(); - // pql 1.5+ returns exit 0 with an empty `[]` for zero matches, so any - // non-zero exit is a real error (older pql used exit 2 for empty). - if (r.exitCode != 0) { - throw PqlException( - 'pql ${args.first} failed', - exitCode: r.exitCode, - stderr: stderr, - ); - } - final stdout = (r.stdout as String).trim(); - if (stdout.isEmpty) return null; - return jsonDecode(stdout); + // Unreachable: the loop returns, continues, or throws on the final attempt. + throw StateError('pql retry loop exhausted without a result'); + } + + /// Whether a non-zero pql exit looks like a transient db-busy / not-yet-ready + /// condition worth retrying, vs. a genuine error to surface immediately. + static bool _isTransient(int exitCode, String stderr) { + if (exitCode == _kBusyExitCode) return true; + final s = stderr.toLowerCase(); + return s.contains('database is locked') || s.contains('db busy') || s.contains('database busy') || s.contains('locked'); } } diff --git a/test/pql/client_test.dart b/test/pql/client_test.dart index 25987ccb..b1acb534 100644 --- a/test/pql/client_test.dart +++ b/test/pql/client_test.dart @@ -150,4 +150,43 @@ void main() { } }); }); + + group('PqlClient — transient retry (T-350)', () { + late Directory tmp; + setUp(() async => tmp = await Directory.systemTemp.createTemp('clide_fakepql_')); + tearDown(() async { + if (await tmp.exists()) await tmp.delete(recursive: true); + }); + + // A fake `pql` whose body is [body]; a fresh `$0.n` counter file per test + // lets a script "recover" after N invocations. + Future fakePql(String body) async { + final f = File('${tmp.path}/pql'); + await f.writeAsString('#!/bin/sh\n$body\n'); + await Process.run('chmod', ['+x', f.path]); + return PqlClient(workDir: Directory.current, toolchain: ToolchainView.resolved(ResolvedPaths(pql: f.path))); + } + + test('a genuine (non-busy) error surfaces immediately', () async { + final p = await fakePql('exit 2'); + await expectLater(p.files(), throwsA(isA().having((e) => e.exitCode, 'exitCode', 2))); + }); + + test('a persistent db-busy (exit 69) throws after exhausting retries', () async { + final p = await fakePql('exit 69'); + await expectLater(p.files(), throwsA(isA().having((e) => e.exitCode, 'exitCode', 69))); + }); + + test('a transient db-busy (exit 69) recovers on retry', () async { + final p = await fakePql(r'c="$0.n"; n=$(cat "$c" 2>/dev/null || echo 0); n=$((n+1)); echo "$n" > "$c"; ' + r'if [ "$n" -lt 3 ]; then exit 69; fi; echo "[]"'); + expect(await p.files(), isEmpty); // retried through two busies to success + }); + + test('a "database is locked" stderr (non-69 exit) is also retried', () async { + final p = await fakePql(r'c="$0.n"; n=$(cat "$c" 2>/dev/null || echo 0); n=$((n+1)); echo "$n" > "$c"; ' + r'if [ "$n" -lt 3 ]; then echo "database is locked" >&2; exit 1; fi; echo "[]"'); + expect(await p.files(), isEmpty); + }); + }); }