review fixes: system logs survive the level gate, writable comes from the device, args parse inside the guard

Author: t0uh33d

lib/src/config/logging_behaviour.dart

@@ -55,6 +55,13 @@ class LoggingBehavior {
   }
 
   bool shouldLog(LogEntry entry) {
+    // System logs are the SDK's own record — a dashboard edit to a local
+    // database, not something the app called. minimumLevel exists to control
+    // the app's volume, and system's value (500) sits below even verbose, so
+    // without this exemption the default config silently drops every one and
+    // an audit line is never written anywhere.
+    if (entry.level == LogLevel.system) return true;
+
     if (entry.level.value < minimumLevel.value) return false;
     if (enabledTags.contains('*')) return true;
 

lib/src/db/db_dispatcher.dart

@@ -1,5 +1,7 @@
+import 'package:code_scout/src/code_scout.dart';
 import 'package:code_scout/src/db/db_registry.dart';
 import 'package:code_scout/src/db/db_source.dart';
+import 'package:code_scout/src/log/log_level.dart';
 
 /// Answers the database questions the dashboard asks over a live session.
 ///
@@ -66,16 +68,39 @@ class DatabaseDispatcher {
           dbName,
           CodeScoutReadRequest.fromJson(args),
         );
-        return {'ok': true, 'page': page.toJson()};
+        // writable rides along so the dashboard's edit affordances come from
+        // this registration rather than from anything the browser said about
+        // itself. The enforcement is in the registry either way; this is about
+        // not drawing buttons that would only ever be refused.
+        return {'ok': true, 'page': page.toJson(), 'writable': entry.writable};
 
       case 'update':
         // Straight to the registry, never to the source: the registry is where
         // writable is enforced, and routing round it would make that flag a
         // suggestion.
-        final result = await DatabaseRegistry.i.write(
-          dbName,
-          CodeScoutWriteRequest.fromJson(args),
-        );
+        final request = CodeScoutWriteRequest.fromJson(args);
+        final result = await DatabaseRegistry.i.write(dbName, request);
+
+        if (result.ok) {
+          // A read leaves no trace and should not. A write changed this
+          // device, so it goes through the ordinary logging pipeline: it lands
+          // in the session timeline, syncs with everything else, and answers
+          // "why is this device's data odd" three weeks later. No audit table.
+          final by = args['by'] as String? ?? 'the dashboard';
+          CodeScout.instance.log(
+            level: LogLevel.system,
+            message:
+                'Dashboard edited $dbName ${request.namespace}.${request.column} on ${request.handle} · by $by',
+            tags: {'codescout'},
+            metadata: {
+              'db': dbName,
+              'namespace': request.namespace,
+              'column': request.column,
+              'handle': '${request.handle}',
+              'by': by,
+            },
+          );
+        }
         return {...result.toJson()};
 
       default:

lib/src/db/sqflite_source.dart

@@ -120,8 +120,13 @@ class CodeScoutSqflite implements CodeScoutSource {
       // CAST so one rule covers every type: a number filtered by "42" behaves
       // the way the person typing it expects, with no per-type branching and
       // no expression language to parse.
-      where.add('CAST(${quoteIdentifier(column)} AS TEXT) LIKE ?');
-      args.add('%$text%');
+      //
+      // The typed text is escaped so % and _ mean the characters, not LIKE's
+      // wildcards. Somebody filtering a discounts column by "100%" wants rows
+      // containing that string, not every row.
+      final escaped = text.replaceAll(r'\', r'\\').replaceAll('%', r'\%').replaceAll('_', r'\_');
+      where.add("CAST(${quoteIdentifier(column)} AS TEXT) LIKE ? ESCAPE '\\'");
+      args.add('%$escaped%');
     });
 
     final limit = request.limit.clamp(1, maxPageRows);

lib/src/live/live_session_client.dart

@@ -197,11 +197,14 @@ class LiveSessionClient extends ChangeNotifier {
   /// gets no reply waits out its full timeout and then cannot tell a broken
   /// query from a phone that went to sleep.
   Future<void> _answer(String req, Map<String, dynamic> message) async {
-    final op = message['op'] as String? ?? '';
-    final args = (message['args'] as Map?)?.cast<String, dynamic>() ?? <String, dynamic>{};
-
+    // Parsing happens inside the try. `args` of the wrong shape entirely — a
+    // string where a map should be — would otherwise throw before the guard,
+    // become an unhandled async error, and send no reply at all: the worst of
+    // both worlds.
     Map<String, dynamic> body;
     try {
+      final op = message['op'] as String? ?? '';
+      final args = (message['args'] as Map?)?.cast<String, dynamic>() ?? <String, dynamic>{};
       body = await DatabaseDispatcher.handle(op, args);
     } catch (e) {
       body = {'ok': false, 'error': e.toString()};

test/db_dispatcher_test.dart

@@ -2,6 +2,7 @@ import 'dart:convert';
 import 'dart:typed_data';
 
 import 'package:code_scout/code_scout.dart';
+import 'package:code_scout/src/csx_interface/log_buffer.dart';
 import 'package:code_scout/src/db/db_dispatcher.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:sqflite_common_ffi/sqflite_ffi.dart';
@@ -82,6 +83,14 @@ void main() {
   group('rows', () {
     setUp(() => DatabaseRegistry.i.register('shop.db', CodeScoutSqflite(db)));
 
+    test('rows carries writable from the registration, not from the caller', () async {
+      // The dashboard draws edit affordances off this. It must come from what
+      // the app agreed to, never from anything the browser said about itself.
+      final reply = await DatabaseDispatcher.handle(
+          'rows', {'db': 'shop.db', 'namespace': 'flags'});
+      expect(reply['writable'], isFalse);
+    });
+
     test('a page comes back with columns, rows and handles', () async {
       final reply = await DatabaseDispatcher.handle(
           'rows', {'db': 'shop.db', 'namespace': 'flags'});
@@ -148,6 +157,47 @@ void main() {
       expect((await db.rawQuery('SELECT enabled FROM flags')).first['enabled'], 0);
     });
 
+    test('a successful write is logged on the device', () async {
+      // A read leaves no trace and should not. A write changed the device, so
+      // it goes through the ordinary pipeline and lands in the session
+      // timeline — the answer to "why is this device's data odd" weeks later.
+      DatabaseRegistry.i.register('shop.db', CodeScoutSqflite(db), writable: true);
+      LogBuffer.i.clear();
+
+      await DatabaseDispatcher.handle('update', {
+        'db': 'shop.db',
+        'namespace': 'flags',
+        'column': 'enabled',
+        'handle': await rowid(),
+        'value': '1',
+        'was': 0,
+        'by': 'ada@example.com',
+      });
+
+      final audit = LogBuffer.i.entries.where((e) => e.level == LogLevel.system);
+      expect(audit, hasLength(1), reason: 'the write left no trace');
+      expect(audit.single.message, contains('flags.enabled'));
+      expect(audit.single.message, contains('ada@example.com'));
+    });
+
+    test('a refused write is not logged as a change', () async {
+      // Nothing changed, so nothing happened worth recording. An audit line
+      // for a refusal would read as "the dashboard edited this" when it did not.
+      DatabaseRegistry.i.register('shop.db', CodeScoutSqflite(db));
+      LogBuffer.i.clear();
+
+      await DatabaseDispatcher.handle('update', {
+        'db': 'shop.db',
+        'namespace': 'flags',
+        'column': 'enabled',
+        'handle': await rowid(),
+        'value': '1',
+        'was': 0,
+      });
+
+      expect(LogBuffer.i.entries.where((e) => e.level == LogLevel.system), isEmpty);
+    });
+
     test('a conflict carries the current value back', () async {
       DatabaseRegistry.i.register('shop.db', CodeScoutSqflite(db), writable: true);
       await db.rawUpdate('UPDATE flags SET enabled = 9');

test/db_query_test.dart

@@ -198,6 +198,19 @@ void main() {
       expect(page.rows.first.first.display, 'checkout_one_tap');
     });
 
+    test("LIKE's wildcards mean themselves in a filter", () async {
+      // Somebody filtering by "100%" wants rows containing that string, not
+      // every row with a 1 followed by two zeroes and anything at all.
+      await db.insert('flags', {'key': 'discount_100%', 'enabled': 0});
+      await db.insert('flags', {'key': 'discount_100x', 'enabled': 0});
+
+      final page = await source.read(
+        const CodeScoutReadRequest(namespace: 'flags', filters: {'key': '100%'}),
+      );
+      expect(page.rows, hasLength(1));
+      expect(page.rows.single.first.display, 'discount_100%');
+    });
+
     test('an empty filter is not a filter', () async {
       final page = await source.read(
         const CodeScoutReadRequest(namespace: 'flags', filters: {'key': ''}),
@@ -219,7 +232,7 @@ void main() {
       expect(second.hasMore, isFalse);
     });
 
-    test('the row ceiling is the deviceies, not the dashboard\'s', () async {
+    test("the row ceiling is the device's, not the dashboard's", () async {
       // A limit the caller can raise is not a limit. Only the device knows what
       // it costs to build the page.
       for (var i = 0; i < 150; i++) {