diff --git a/WORKLOG.md b/WORKLOG.md index 6758e44ee..9f2c276e3 100644 --- a/WORKLOG.md +++ b/WORKLOG.md @@ -1,5 +1,46 @@ # Worklog +## 2026-06-26 (lint fixes) + +### nx2/blocks/chat — CI lint fixes + +- `chat-controller.js` line 283: broke the long `.then().catch()` chain onto three lines to satisfy `max-len` (100 char limit). +- `persistence.js` line 119: changed bare `return;` to `return undefined;` so the `.then()` callback is `consistent-return` (always returns a value or always returns nothing — now always returns a value). +- `persistence.js` line 123: replaced `console.warn(...)` on `tx.onerror` with a silent no-op comment; transaction errors bubble via `req.onerror → reject(err)` anyway. + +## 2026-06-26 (follow-up) + +### nx2/blocks/chat — review fixes for always-approve persistence + +**persistence.js**: +- `saveAutoApprovedTools` now returns a real Promise (resolves on `tx.oncomplete`, rejects on `tx.onerror` / `req.onerror` / `store.put` throw). Previously `async` function returned `undefined` before the IDB write settled. +- Added `blankRecord(room)` factory to replace duplicated `{ room, messages: [], sessionId: null }` skeleton. +- `tx.onerror` now logs a `console.warn` instead of silently swallowing quota-exceeded errors. + +**chat-controller.js**: +- `approveToolCall`: removed dead `??= new Set()` guard (unreachable after `loadInitialMessages` always sets `_autoApprovedTools`). +- Fire-and-forget `saveAutoApprovedTools` call now has `.catch(() => {})` to suppress unhandled rejection. +- Added comment in `clear()` explaining `_autoApprovedTools` is intentionally not reset (persisted per-user). + +**Tests**: removed `setTimeout(50)` timing hacks — tests now `await saveAutoApprovedTools(...)` directly. + +## 2026-06-26 + +### nx2/blocks/chat — persist always-approve tool selections (feat/da-always-approve-persist branch) + +`_autoApprovedTools` was an in-memory Set that reset on every page load. Now it round-trips through IndexedDB alongside messages. + +**persistence.js** — two new exports: +- `loadAutoApprovedTools(room)` — readonly transaction, returns `new Set(record?.autoApprovedTools ?? [])`. +- `saveAutoApprovedTools(room, toolNamesSet)` — opens a readwrite transaction, does a `get` then `put` inside the same transaction to merge only `autoApprovedTools` without touching `messages` or `sessionId`. + +**chat-controller.js**: +- `loadInitialMessages` now calls both `loadMessages` and `loadAutoApprovedTools` in parallel via `Promise.all` and sets `this._autoApprovedTools` from the result. +- `approveToolCall` fires `saveAutoApprovedTools` (fire-and-forget) after adding to the Set in the `if (always)` branch. +- `clear()` no longer resets `_autoApprovedTools` — the Set persists through conversation resets. + +Tests: 4 new cases in `test/nx2/blocks/chat/utils/persistence.test.js`; all 28 chat tests pass. + ## 2026-06-26 ### nx2/blocks/chat/chat.js — skill selection preserves pending attachments (feat/da-skill-attachment-fix) @@ -14,7 +55,6 @@ Post-review follow-up (fe049a9b): - Renamed loop variable `i` → `item` in `_onSlashSelect` callbacks - Added regression tests in `test/nx2/blocks/chat/chat.test.js` (8 tests, all pass) - ## 2026-06-23 ### nx2/blocks/shared/dialog — configurable panel sizing (dialog-css-vars branch) diff --git a/nx2/blocks/chat/chat-controller.js b/nx2/blocks/chat/chat-controller.js index 02da447a1..0f69971d7 100644 --- a/nx2/blocks/chat/chat-controller.js +++ b/nx2/blocks/chat/chat-controller.js @@ -1,7 +1,10 @@ import { loadIms } from '../../utils/ims.js'; import { AGENT_EVENT, ROLE, TOOL_NAME, TOOL_STATE } from './constants.js'; import { readStream } from './utils/stream.js'; -import { loadMessages, saveMessages, resetSession, getRoomKey } from './utils/persistence.js'; +import { + loadMessages, saveMessages, resetSession, getRoomKey, + loadAutoApprovedTools, saveAutoApprovedTools, +} from './utils/persistence.js'; function affectedFolders(toolName, input) { const { org, repo } = input ?? {}; @@ -104,7 +107,11 @@ export default class ChatController { async loadInitialMessages() { this._messages = []; const room = await this._getRoom(); - const { messages: cached, sessionId } = await loadMessages(room); + const [{ messages: cached, sessionId }, autoApprovedTools] = await Promise.all([ + loadMessages(room), + loadAutoApprovedTools(room), + ]); + this._autoApprovedTools = autoApprovedTools; this._sessionId = sessionId ?? this._sessionId; if (!cached.length) return; this._messages = stripOrphanedToolCallMessages(cached); @@ -162,9 +169,10 @@ export default class ChatController { this._messages = undefined; this._streamingText = undefined; this._toolCards = new Map(); - this._autoApprovedTools = new Set(); this._sessionId = crypto.randomUUID(); this._currentTurnId = crypto.randomUUID(); + // _autoApprovedTools is intentionally NOT reset here: it is persisted per-user + // in IndexedDB and should survive conversation clears. this._update(); const room = await this._getRoom(); resetSession(room, this._sessionId); @@ -271,8 +279,10 @@ export default class ChatController { if (!card?.approvalId) return; if (always) { - this._autoApprovedTools ??= new Set(); this._autoApprovedTools.add(card.toolName); + this._getRoom() + .then((room) => saveAutoApprovedTools(room, this._autoApprovedTools)) + .catch(() => {}); } const next = new Map(this._toolCards ?? []); diff --git a/nx2/blocks/chat/utils/persistence.js b/nx2/blocks/chat/utils/persistence.js index b58a4107c..d14bdb476 100644 --- a/nx2/blocks/chat/utils/persistence.js +++ b/nx2/blocks/chat/utils/persistence.js @@ -3,6 +3,10 @@ const DB_VERSION = 1; const STORE_NAME = 'conversations'; const EMPTY_STATE = { messages: [], sessionId: null }; +function blankRecord(room) { + return { room, messages: [], sessionId: null, updatedAt: Date.now() }; +} + let dbPromise = null; function closeDb(resolve) { @@ -89,6 +93,52 @@ export function resetSession(room, sessionId) { return write((store) => store.put({ room, messages: [], sessionId, updatedAt: Date.now() })); } +export async function loadAutoApprovedTools(room) { + const db = await openDb(); + if (!db) return new Set(); + + return new Promise((resolve) => { + try { + const tx = db.transaction(STORE_NAME, 'readonly'); + const req = tx.objectStore(STORE_NAME).get(room); + + req.onsuccess = (e) => { + const { result } = e.target; + resolve(new Set(Array.isArray(result?.autoApprovedTools) ? result.autoApprovedTools : [])); + }; + req.onerror = () => resolve(new Set()); + } catch { + resolve(new Set()); + } + }); +} + +export function saveAutoApprovedTools(room, toolNamesSet) { + return openDb().then((db) => { + if (!db) return undefined; + return new Promise((resolve, reject) => { + try { + const tx = db.transaction(STORE_NAME, 'readwrite'); + tx.oncomplete = () => resolve(); + tx.onerror = () => { /* transaction errors are surfaced via reject in req.onerror */ }; + const store = tx.objectStore(STORE_NAME); + const req = store.get(room); + req.onsuccess = (e) => { + try { + const existing = e.target.result ?? blankRecord(room); + store.put({ ...existing, autoApprovedTools: [...toolNamesSet], updatedAt: Date.now() }); + } catch (err) { + reject(err); + } + }; + req.onerror = (e) => reject(e); + } catch (err) { + reject(err); + } + }); + }); +} + export function getRoomKey({ org, site, userId }) { return org && site && userId ? `${org}--${site}--${userId}` : 'default'; } diff --git a/test/nx2/blocks/chat/utils/persistence.test.js b/test/nx2/blocks/chat/utils/persistence.test.js index c107378f0..01834556f 100644 --- a/test/nx2/blocks/chat/utils/persistence.test.js +++ b/test/nx2/blocks/chat/utils/persistence.test.js @@ -3,6 +3,8 @@ import { loadMessages, saveMessages, resetSession, + loadAutoApprovedTools, + saveAutoApprovedTools, } from '../../../../../nx2/blocks/chat/utils/persistence.js'; let counter = 0; @@ -100,4 +102,48 @@ describe('persistence', () => { expect(result.sessionId).to.equal(id); }); }); + + describe('loadAutoApprovedTools', () => { + it('returns an empty Set for an unknown room', async () => { + const result = await loadAutoApprovedTools(room()); + expect(result).to.be.instanceOf(Set); + expect(result.size).to.equal(0); + }); + + it('returns the saved tool names as a Set', async () => { + const testRoom = room(); + await saveAutoApprovedTools(testRoom, new Set(['content_read', 'content_replace'])); + + const result = await loadAutoApprovedTools(testRoom); + expect(result).to.be.instanceOf(Set); + expect(result.has('content_read')).to.be.true; + expect(result.has('content_replace')).to.be.true; + expect(result.size).to.equal(2); + }); + }); + + describe('saveAutoApprovedTools', () => { + it('does not overwrite existing messages or sessionId', async () => { + const testRoom = room(); + const id = crypto.randomUUID(); + const msgs = [{ role: 'user', content: 'keep me' }]; + await saveMessages(testRoom, msgs, id); + + await saveAutoApprovedTools(testRoom, new Set(['content_read'])); + + const result = await loadMessages(testRoom); + expect(result.messages).to.deep.equal(msgs); + expect(result.sessionId).to.equal(id); + }); + + it('merges new tools into an existing autoApprovedTools list', async () => { + const testRoom = room(); + await saveAutoApprovedTools(testRoom, new Set(['content_read'])); + await saveAutoApprovedTools(testRoom, new Set(['content_read', 'content_replace'])); + + const result = await loadAutoApprovedTools(testRoom); + expect(result.has('content_read')).to.be.true; + expect(result.has('content_replace')).to.be.true; + }); + }); });