Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions docs/evidence/issue131/review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Preserved Fleet/session work review — issue #131

Review date: 2026-09-06. Starting main:
`1c933a39e452b3936afc605173f8c384a565c2c0`.
Preserved branch tip: `103e70bb49aebd422107f0c0183af55768807ce4`
(`fix/chat-scroll-follow-tail`). This is a behavior review of its thirteen commits;
the branch is not an integration batch or a list of thirteen current defects.

## Dispositions

| Preserved commit | Current finding and disposition |
| --- | --- |
| `2084ad0` — follow tail | Already adopted and extended by #99. The current mounted `useChatSessionState.test.ts` covers delayed layout, manual detachment, and live-refresh reattachment. No second scroll implementation. |
| `53cf0dc` — only tmux rows / remove host filter | Not carried forward. Fleet RFC revision 2 retains a bounded recent-session catalog, including history, and explicit host filtering remains useful. Hiding all non-live history would change the shipped catalog behavior. |
| `5fa379b` — mapped pane transcripts | Keep verified terminal fallback. The public pane's provider-native ID is not proof of a unique app-session binding across providers. Do not infer a writable transcript from ID equality. Catalogued app-session rows continue to open their own host-qualified transcripts. |
| `c5b2636` — clear terminal before navigation | Reproduced and corrected in #135. Remote row activation now clears terminal/transcript takeover through the app-shell controller, including selection of an already-active route. |
| `b2e3fdc` — retain idle/uncatalogued transcripts | Do not synthesize missing catalog rows. The RFC explicitly limits omitted rows to the peer's direct UI. #135 additionally removes the fallback from a missing peer selection to the last local project/session. |
| `e957f21` — extra remote row controls | Not carried forward. Keep sidebar rows as target selectors and use the existing verified transcript/terminal control surfaces. No current defect requires the additional mutation entry points in that historical UI proposal. |
| `1d2fbf8` — transient GJC probes | Reproduced and corrected in #134. Both discovery classifiers exclude the exact `gjc skills` utility subcommand; native and Bun/Node interactive/resume forms remain supported. |
| `06875a1` — unchanged heartbeat | Reproduced and corrected in #134 for host-state frames. Identical descriptors preserve catalog identity; label, state, capabilities, and protocol changes still propagate. Full snapshots retain their existing replacement/resync semantics. |
| `b02502c` — catalog/watcher bounds | Session reads are already capped at 512, wire catalogs apply the RFC's priority/omission bounds, and #128 supplies bounded/fair indexing admission and recovery. Do not import a hard 100-project limit, blanket temporary-directory exclusion, or native-watcher rewrite limited to the current Codex year; those changes can omit legitimate data. Preserve the current historical-record coverage and recorded omitted-row policy. |
| `031b2c4` — remote project names | The current browser reads `projects[].displayName` from the owning catalog. `remoteRouteSelection.test.ts` verifies that name on the selected peer. A duplicate `projectName` metadata field has no required consumer; it is not added. |
| `da740ce` — OpenCode database watch | Recursive cache watching was reproduced. The direct-file-only historical approach also missed deletion/replacement events for an initially existing database. The correction keeps the parent directory, uses depth zero, and accepts only that root and its `opencode.db`. Real filesystem tests cover present/missing databases, change, removal, recreation, and excluded cache subtrees. |
| `7c9cf15` — long-lived reads | Superseded by #77. The current `FleetRequestLedger` evicts completed reads and terminal streaming operations immediately while retaining mutation results. Existing mutation-ledger tests cover in-flight coalescing and sustained traffic; do not reintroduce a read-result retention/eviction scheme. |
| `103e70b` — local-only API guards | Reproduced and corrected in #133 for ordinary file mentions, token usage, file links/editor, palette reads and Git actions, including stale callbacks/responses. The relay-file guard already exists through #111. The proposed extra identity route is unnecessary for this correction: the existing authoritative roster adopts the installation ID through `adoptLocalHostIdentity`. |

All six review groups in #131 have a disposition. Retained fixes are separate
changes based on main; the rejected UI/catalog proposals are not silently marked
as implemented. The preserved branch remains available for historical recovery.

## Verification evidence

- #133: two mounted regressions failed on the baseline; corrected full
`npm run verify` passed 2,382 repository tests plus audit, type checks, Rust
checks/tests, lint, identity, and build. See [local API evidence](README.md).
- #134: the probe and heartbeat regressions failed before correction; the
125-test provider group and 13-test catalog group pass, with type checks/lint.
- #135: production sidebar/controller clicks reproduced retained terminal state;
the 15-test navigation/sidebar group passes, with type checks/lint. See
[navigation evidence](navigation.md).
- OpenCode: recursive-cache and direct-file replacement failures were observed;
the corrected filesystem/watcher/indexing group passes 20 tests, with type
checks/lint. No synchronizer cursor, provider scope, or tmux authority changes.

The browser evidence uses production components with synthetic data and no real
peer connections. It does not claim release-grade CUA, SSH installation, physical
phone validation, a new public release, or a running-service deployment. Every
retained change still goes through the required latest-main Node 22/24 and
canonical-bundle PR checks before squash merge.
17 changes: 14 additions & 3 deletions server/modules/providers/services/sessions-watcher.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import os from 'node:os';
import path from 'node:path';
import { promises as fsPromises } from 'node:fs';

import chokidar, { type FSWatcher } from 'chokidar';
import chokidar, { type ChokidarOptions, type FSWatcher } from 'chokidar';

import { projectsDb, sessionsDb } from '@/modules/database/index.js';
import { generateDisplayName } from '@/modules/projects/index.js';
Expand Down Expand Up @@ -72,6 +72,18 @@ const WATCHER_IGNORED_PATTERNS = [
'**/.DS_Store',
];

export function sessionWatchOptions(provider: LLMProvider, rootPath: string): Pick<ChokidarOptions, 'ignored' | 'depth'> {
if (provider !== 'opencode') return { ignored: WATCHER_IGNORED_PATTERNS, depth: 6 };
const root = path.resolve(rootPath);
const database = path.join(root, 'opencode.db');
// Keep the parent directory to observe database creation/replacement, but
// never recurse into caches or watch unrelated files beside the database.
return { depth: 0, ignored: (candidate) => {
const absolute = path.resolve(candidate);
return absolute !== root && absolute !== database;
} };
}

const PROJECTS_UPDATE_DEBOUNCE_MS = 500;
const PROJECTS_UPDATE_MAX_WAIT_MS = 2_000;
const WATCHER_FALLBACK_RECONCILE_MS = 60_000;
Expand Down Expand Up @@ -613,11 +625,10 @@ export async function initializeSessionsWatcher(): Promise<void> {
if (sessionWatchersClosing || indexingScheduler !== scheduler) return;

const watcher = chokidar.watch(rootPath, {
ignored: WATCHER_IGNORED_PATTERNS,
...sessionWatchOptions(provider, rootPath),
persistent: true,
ignoreInitial: true,
followSymlinks: false,
depth: 6,
usePolling: process.env.CHATMUX_SESSION_WATCH_POLLING === '1',
interval: 6_000,
binaryInterval: 6_000,
Expand Down
45 changes: 45 additions & 0 deletions server/modules/providers/tests/opencode-watcher-scope.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import assert from 'node:assert/strict';
import { once } from 'node:events';
import { mkdtemp, mkdir, rm, unlink, writeFile } from 'node:fs/promises';
import os from 'node:os';
import path from 'node:path';
import test from 'node:test';

import chokidar from 'chokidar';

import { sessionWatchOptions } from '@/modules/providers/services/sessions-watcher.service.js';

for (const initialDatabase of [true, false]) {
test(`OpenCode watches only its database and recovers replacement (initial database: ${initialDatabase})`, { timeout: 15_000 }, async (t) => {
const root = await mkdtemp(path.join(os.tmpdir(), 'chatmux-watch-scope-'));
const database = path.join(root, 'opencode.db');
await mkdir(path.join(root, 'cache', 'nested'), { recursive: true });
await writeFile(path.join(root, 'cache', 'nested', 'cache.jsonl'), 'fixture');
if (initialDatabase) await writeFile(database, 'fixture');
const watcher = chokidar.watch(root, {
ignoreInitial: true, followSymlinks: false, ...sessionWatchOptions('opencode', root),
});
t.after(async () => { await watcher.close(); await rm(root, { recursive: true, force: true }); });
await once(watcher, 'ready', { signal: AbortSignal.timeout(5_000) });
assert.ok(!Object.keys(watcher.getWatched()).some((directory) => directory.includes(`${path.sep}cache`)));
const update = once(watcher, initialDatabase ? 'change' : 'add', { signal: AbortSignal.timeout(5_000) });
await writeFile(database, 'updated fixture database');
assert.equal((await update)[0], database);
const removed = once(watcher, 'unlink', { signal: AbortSignal.timeout(5_000) });
await unlink(database);
assert.equal((await removed)[0], database);
const recreated = once(watcher, 'add', { signal: AbortSignal.timeout(5_000) });
await writeFile(database, 'replacement database');
assert.equal((await recreated)[0], database);
assert.ok(!Object.keys(watcher.getWatched()).some((directory) => directory.includes(`${path.sep}cache`)));
});
}

test('directory-based providers retain recursive watching and cache exclusions', () => {
for (const provider of ['claude', 'codex', 'cursor', 'omp', 'omo', 'gjc'] as const) {
const options = sessionWatchOptions(provider, '/fixture/root');
assert.equal(options.depth, 6);
assert.ok(Array.isArray(options.ignored));
assert.ok(options.ignored.includes('**/node_modules/**'));
}
});
Loading