From 19051db5d62ebe0d651a9789bf495eb1e5256e10 Mon Sep 17 00:00:00 2001 From: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Date: Tue, 22 Sep 2026 01:14:40 +0800 Subject: [PATCH 1/4] fix(ui-web): the rail's import row is the work outstanding, not a receipt The row above Settings was drawn from the importer's state file alone. That file outlives every run that wrote it, so any install whose file is not empty greeted its next reader with "memory import finished", whether or not that reader had ever started an import or seen the onboarding wizard. A run the CLI started records no request, so its failure count carries no retry either: the row said "1 did not make it" and the click was dead. A settled run, finished or stopped short, now earns a row only when one of two things holds: its click can still act on it, or this page followed the run, by asking for one or by finding one already in flight. Everything live (the scan, the run, the wrapping phases) is unchanged. The wizard's data-sync step now asks for its run through the same store verb the rail's own click uses instead of calling import.run a second way, so a run started from the wizard is one the page is following. Co-authored-by: Claude (claude-opus-5[1m]) --- ui-web/src/app/install.test.ts | 34 +++++++++ ui-web/src/app/install.ts | 8 ++- .../importSync/ImportSyncPage.test.tsx | 13 +++- ui-web/src/features/importSync/store.test.ts | 70 ++++++++++++++++++- ui-web/src/features/importSync/store.ts | 52 ++++++++++---- 5 files changed, 159 insertions(+), 18 deletions(-) diff --git a/ui-web/src/app/install.test.ts b/ui-web/src/app/install.test.ts index 0f5035a27..07811d316 100644 --- a/ui-web/src/app/install.test.ts +++ b/ui-web/src/app/install.test.ts @@ -107,3 +107,37 @@ describe('the session-pointer reload (installActions)', () => { _resetForTests() }) }) + +/* The wizard's data-sync step and the rail's own click are one verb, so the + store is following the run either way -- which is what decides whether its + finished row is drawn (features/importSync/store.ts's `stale`). */ +describe('the wizard asks for its import through the rail row (installSources)', () => { + it('routes startImport to the store, so the run is one this page follows', async () => { + const wiring = await harness() + const { ds, setSources } = await import('../state/sources') + const store = await import('../features/importSync/store') + const runs: Array<[string[], string]> = [] + setSources({ composer: { slash: [] } } as unknown as Partial) + + wiring.installSources() + setSources({ + importSync: { + status: async () => ({ + running: false, total: 1, submitted: 1, failed: 0, by_platform: {}, + phase: null, phases: null, tier: 'memory_files', platforms: ['hermes'], + }), + run: async (platforms: string[], tier: string) => { + runs.push([platforms, tier]) + return { started: true, total: 1, detail: '' } + }, + stop: async () => ({ stopped: true }), + }, + } as unknown as Partial) + + await ds('onboard').startImport(['hermes'], 'memory_files') + + expect(runs).toEqual([[['hermes'], 'memory_files']]) + expect(store.get().watched).toBe(true) + store._resetForTests() + }) +}) diff --git a/ui-web/src/app/install.ts b/ui-web/src/app/install.ts index 7d8d451f9..a5eeb7e15 100644 --- a/ui-web/src/app/install.ts +++ b/ui-web/src/app/install.ts @@ -148,7 +148,13 @@ export function installSources(): void { sources.cron = cronSource sources.connections = connSource sources.memory = memorySource - sources.onboard = onboardSource + /* The wizard's data-sync step asks for the same run the rail's own click + does, so it goes through the same verb rather than calling `import.run` a + second way: the store is then following a run it knows is its own, which is + what keeps the finished row on the rail (features/importSync/store.ts's + `stale`). Grown here rather than by the wizard importing a sibling + domain's store, the same way the step bodies below are handed over. */ + sources.onboard = { ...onboardSource, startImport: (platforms, tier) => importSyncStore.start(platforms, tier) } /* The onboarding wizard draws the owning domains' own bodies for its three configured steps -- the settings dialog's model page and web controls, the sub-agents roster -- and asks each domain's store whether the step is diff --git a/ui-web/src/features/importSync/ImportSyncPage.test.tsx b/ui-web/src/features/importSync/ImportSyncPage.test.tsx index c5c71357e..795faf793 100644 --- a/ui-web/src/features/importSync/ImportSyncPage.test.tsx +++ b/ui-web/src/features/importSync/ImportSyncPage.test.tsx @@ -38,8 +38,10 @@ afterEach(() => { localStorage.clear() }) -const draw = (st: ImportStatus | null): void => { - store.set({ status: st }) +/* Drawn as a run this page followed, which is what every case below is about; + the one case about a run it did not follow turns `watched` off itself. */ +const draw = (st: ImportStatus | null, patch: Partial = {}): void => { + store.set({ status: st, watched: true, ...patch }) render() } const row = (): HTMLElement | null => document.querySelector('.importSync') @@ -113,6 +115,13 @@ describe('the import row', () => { expect(x()).toBeNull() }) + /* An import somebody ran from the CLI weeks ago is still in the importer's + file. It is not this reader's news and its count has no retry behind it. */ + it('draws nothing for a finished run this page never followed', () => { + draw(status({ total: 2, submitted: 1, failed: 1, platforms: ['claude_code'] }), { watched: false }) + expect(row()).toBeNull() + }) + it('counts a failed phase among what did not make it', () => { draw(status({ total: 2, submitted: 2, tier: 'full', platforms: ['hermes'], phases: { status: 'failed', errors: ['profile: bad byte'] } })) expect(row()?.textContent).toContain('gui.importSync.failed_n:{"n":1}') diff --git a/ui-web/src/features/importSync/store.test.ts b/ui-web/src/features/importSync/store.test.ts index 2feb8d9b7..405d87f65 100644 --- a/ui-web/src/features/importSync/store.test.ts +++ b/ui-web/src/features/importSync/store.test.ts @@ -10,8 +10,12 @@ const status = (patch: Partial = {}): ImportStatus => ({ running: false, total: 0, submitted: 0, failed: 0, by_platform: {}, phase: null, phases: null, tier: null, platforms: [], ...patch, }) +/* `watched` defaults on: every case below is about a run this page asked for + or found in flight, which is the only kind that has ever drawn a settled + row. The cases that turn it off are the ones about a run left in the + importer's file by somebody else. */ const state = (st: ImportStatus | null, patch: Partial = {}): store.ImportSyncState => ({ - status: st, starting: false, dismissed: '', error: '', ...patch, + status: st, starting: false, watched: true, dismissed: '', error: '', ...patch, }) interface Fake { @@ -239,9 +243,11 @@ describe('following a run', () => { it('dismiss remembers the finished run across a reload', async () => { const finished = status({ total: 4, submitted: 4, tier: 'full', platforms: ['hermes'], phases: { status: 'done', errors: [] } }) - const f = fake([finished]) + /* Followed from in flight to settled, which is how a row gets there. */ + const f = fake([status({ running: true, total: 4, submitted: 1, tier: 'full', platforms: ['hermes'] }), finished]) setSources({ importSync: f.src }) await store.refresh() + await store.refresh() expect(store.view(store.get()).kind).toBe('done') store.dismiss() @@ -249,4 +255,64 @@ describe('following a run', () => { expect(store.view(store.get()).kind).toBe('hidden') expect(localStorage.getItem('raven.importSync.dismissed')).toBe(store.signature(finished)) }) + + /* The importer's file outlives the run that wrote it, so a page that never + asked for a run still finds one there. */ + it('adopts a run it finds already in flight, and keeps its finished row', async () => { + const f = fake([ + status({ running: true, total: 2, submitted: 1, tier: 'full', platforms: ['hermes'] }), + status({ total: 2, submitted: 2, tier: 'full', platforms: ['hermes'], phases: { status: 'done', errors: [] } }), + ]) + setSources({ importSync: f.src }) + await store.refresh() + expect(store.view(store.get()).kind).toBe('run') + await store.refresh() + expect(store.view(store.get()).kind).toBe('done') + }) + + it('draws nothing for a settled run it never followed', async () => { + const f = fake([status({ total: 2, submitted: 2, tier: 'full', platforms: ['hermes'], phases: { status: 'done', errors: [] } })]) + setSources({ importSync: f.src }) + await store.refresh() + expect(store.view(store.get()).kind).toBe('hidden') + }) + + it('follows the run it asked for itself, so the finish is drawn', async () => { + const f = fake([status({ total: 2, submitted: 1, failed: 1 })]) + setSources({ importSync: f.src }) + await store.start(['hermes'], 'full') + expect(store.view(store.get()).kind).toBe('done') + }) +}) + +/* The row is the work outstanding, never a receipt: a run nobody here watched + is on the rail only while its click can still do something. */ +describe('a run left in the file by somebody else', () => { + const unwatched = { watched: false } + + it('is not drawn when it finished and nothing can be retried', () => { + const cli = status({ total: 2, submitted: 2, platforms: ['claude_code', 'hermes'] }) + expect(store.view(state(cli, unwatched)).kind).toBe('hidden') + }) + + /* The row this reports: a CLI run records no request, so its failure count + has no retry behind it and the reader can do nothing but dismiss it. */ + it('is not drawn when it finished with failures it cannot ask for again', () => { + const cli = status({ total: 2, submitted: 1, failed: 1, platforms: ['claude_code', 'hermes'] }) + expect(store.view(state(cli, { watched: true }))) + .toMatchObject({ kind: 'done', failed: 1, clickable: false }) + expect(store.view(state(cli, unwatched)).kind).toBe('hidden') + }) + + it('is not drawn when it stopped short and cannot be resumed', () => { + expect(store.view(state(status({ total: 4, submitted: 2 }), unwatched)).kind).toBe('hidden') + }) + + it('is still drawn when the click can act on it', () => { + const asked = { tier: 'full' as const, platforms: ['hermes'] } + expect(store.view(state(status({ total: 4, submitted: 2, ...asked }), unwatched))) + .toMatchObject({ kind: 'paused', clickable: true }) + expect(store.view(state(status({ total: 4, submitted: 3, failed: 1, ...asked, phases: { status: 'done', errors: [] } }), unwatched))) + .toMatchObject({ kind: 'done', failed: 1, clickable: true }) + }) }) diff --git a/ui-web/src/features/importSync/store.ts b/ui-web/src/features/importSync/store.ts index 605cd7bcd..b4e480df8 100644 --- a/ui-web/src/features/importSync/store.ts +++ b/ui-web/src/features/importSync/store.ts @@ -9,9 +9,14 @@ * only while a run is on; otherwise the page reads once at boot and once when * the wizard closes (app/install.ts), the two moments a run can begin. * - * A finished run stays on the rail until dismissed, and the dismissal is - * remembered per run (its request and its final counts) so a reload does not - * bring the same finished row back. */ + * A settled run -- finished, or stopped short -- is on the rail only while + * this page has something to say about it: it asked for the run or found it in + * flight, or the row's click can still act on it. The importer's file outlives + * every run that wrote it, so without that the next reader of any install whose + * file is not empty is told about an import they never started. A finished run + * that did earn its row stays until dismissed, and the dismissal is remembered + * per run (its request and its final counts) so a reload does not bring the + * same finished row back. */ import { ds } from '../../state/sources' import { makeStore } from '../../state/store' @@ -28,6 +33,10 @@ export interface ImportSyncState { /* Between asking for a run and hearing that it started: the scan the gateway does before it answers is the one stretch with no counts to show. */ starting: boolean + /* Whether this page has a run of its own to follow: it asked for one, or it + found one in flight. A settled run is only ever on the rail because of + this -- see `stale` below. */ + watched: boolean dismissed: string error: string } @@ -48,6 +57,15 @@ export interface RowView { const HIDDEN: RowView = { kind: 'hidden', pct: 0, failed: 0, phase: null, source: null, clickable: false } +/* Whether a run that is NOT running has earned a row. Two reasons it has: + something can still be done about it, or this page watched it happen. A run + that is neither is history -- the importer's file outlives every run that + wrote it, so a CLI import from weeks ago, or one an earlier reader started, + would otherwise greet the next reader as news. It is not news: nothing is + moving, the click does nothing, and the reader never asked for it. The row + is the work outstanding, never a receipt for work nobody here watched. */ +const stale = (s: ImportSyncState, clickable: boolean): boolean => !clickable && !s.watched + const readDismissed = (): string => { try { return window.localStorage.getItem(DISMISSED_KEY) ?? '' } catch { return '' } } @@ -56,7 +74,7 @@ const writeDismissed = (sig: string): void => { try { window.localStorage.setItem(DISMISSED_KEY, sig) } catch { /* a private window keeps nothing */ } } -const initial = (): ImportSyncState => ({ status: null, starting: false, dismissed: readDismissed(), error: '' }) +const initial = (): ImportSyncState => ({ status: null, starting: false, watched: false, dismissed: readDismissed(), error: '' }) const store = makeStore(initial()) @@ -100,15 +118,21 @@ export function view(s: ImportSyncState): RowView { } if (!total && !phases) return HIDDEN /* Short of the total: the message pass was stopped, or the gateway lost it. */ - if (settled < total) return { kind: 'paused', pct, failed: st.failed, phase: null, source: null, clickable: again } + if (settled < total) { + return stale(s, again) ? HIDDEN : { kind: 'paused', pct, failed: st.failed, phase: null, source: null, clickable: again } + } /* Settled, but the phases behind the pass never finished: no verdict on file for a run that recorded its request (lost before the phases began), or a verdict that says they were still running or were stopped. */ const unfinished = phases === null ? again : phases.status === 'pending' || phases.status === 'cancelled' - if (unfinished) return { kind: 'paused', pct: 100, failed: st.failed, phase: null, source: null, clickable: again } + if (unfinished) { + return stale(s, again) ? HIDDEN : { kind: 'paused', pct: 100, failed: st.failed, phase: null, source: null, clickable: again } + } const failed = st.failed + (phases?.status === 'failed' ? phases.errors.length : 0) + const retry = failed > 0 && again if (s.dismissed === signature(st)) return HIDDEN - return { kind: 'done', pct: 100, failed, phase: null, source: null, clickable: failed > 0 && again } + if (stale(s, retry)) return HIDDEN + return { kind: 'done', pct: 100, failed, phase: null, source: null, clickable: retry } } const failure = (e: unknown): string => { @@ -127,7 +151,7 @@ function poll(on: boolean): void { export async function refresh(): Promise { try { const status = await source().status() - set({ status, error: '' }) + set({ status, error: '', ...(status.running ? { watched: true } : {}) }) poll(status.running) } catch (e) { set({ error: failure(e) }) @@ -135,11 +159,13 @@ export async function refresh(): Promise { } } -/* Ask for a run and follow it. The wizard's sync step goes through its own - source for the same call and lets the close hook bring this store up to date; - the rail's own click comes here. */ +/* Ask for a run and follow it. Every caller comes here -- the rail's own click + and the wizard's sync step, which reaches it through the seam (src/app/ + install.ts) rather than calling the same method a second way. Asking is what + makes the run this page's to follow, which is what keeps its finished row on + the rail afterwards. */ export async function start(platforms: string[], tier: ImportTier): Promise { - set({ starting: true, error: '' }) + set({ starting: true, watched: true, error: '' }) try { const r = await source().run(platforms, tier) /* Read after, then report: a refusal is this call's news, and the read @@ -180,5 +206,5 @@ export function dismiss(): void { export function _resetForTests(): void { poll(false) store._resetForTests() - store.set({ ...initial(), dismissed: '' }) + store.set({ ...initial(), watched: false, dismissed: '' }) } From 742427a37d7ebc29588d7e8794a1f1e29ce61a39 Mon Sep 17 00:00:00 2001 From: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Date: Tue, 22 Sep 2026 01:14:55 +0800 Subject: [PATCH 2/4] fix(ui-web): a workspace path chip opens the file the answer named A backticked path naming the workspace became a chip whose target was that path made relative to the workspace. The viewer's route used to root a relative path at agent home, which made it resolve; it roots one at the session's working directory now (_workspace_root), so a workspace path outside that directory was re-rooted onto it. Measured on a live gateway, with the file present under the configured workspace: GET /file?path=skills/quick-summarize/SKILL.md&session=s -> 404 /skills/quick-summarize/SKILL.md GET /file?path=/workspace/skills/quick-summarize/SKILL.md&session=s -> 200 The chip carried the first form, so it opened onto "that file is no longer there" for a file that was there. It carries the path as the answer wrote it now. The other half of livePathOf, a file this session changed, has always answered with the path as written; shortening for display is the panel's own job (shortPath), not this resolver's. Co-authored-by: Claude (claude-opus-5[1m]) --- ui-web/src/features/workspace/source.test.ts | 16 +++++++++++++--- ui-web/src/features/workspace/source.ts | 13 +++++++++++-- ui-web/src/lib/prose.ts | 7 ++++--- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/ui-web/src/features/workspace/source.test.ts b/ui-web/src/features/workspace/source.test.ts index 51e22271c..68d41102d 100644 --- a/ui-web/src/features/workspace/source.test.ts +++ b/ui-web/src/features/workspace/source.test.ts @@ -79,14 +79,24 @@ describe('whether the gateway host is the reader own desktop', () => { }) describe('a bare path in prose', () => { - it('links a workspace path, relative to the workspace', () => { - expect(livePathOf('/home/a/.raven/workspace/notes.md')).toBe('notes.md') + /* As written, not made relative: the viewer's route roots a relative path at + the session's working directory, so a shortened workspace path opened onto + a file that is not there (the chip said "no longer there" for a file that + was). */ + it('links a workspace path as the answer wrote it', () => { + expect(livePathOf('/home/a/.raven/workspace/notes.md')).toBe('/home/a/.raven/workspace/notes.md') + }) + + /* A workspace path an answer wrote relative stays relative -- the route + roots it where the turn ran, which is what the author meant. */ + it('leaves a relative workspace path alone', () => { + expect(livePathOf('workspace/notes.md')).toBe('workspace/notes.md') }) /* A line and column reference is how a tool names a place in a file; the file is what a click can open. */ it('drops a line and column reference before resolving', () => { - expect(livePathOf('/srv/workspace/a.ts:12:3')).toBe('a.ts') + expect(livePathOf('/srv/workspace/a.ts:12:3')).toBe('/srv/workspace/a.ts') }) it('refuses anything with whitespace in it, and the empty string', () => { diff --git a/ui-web/src/features/workspace/source.ts b/ui-web/src/features/workspace/source.ts index 5ad97a8d8..5f6d52fd1 100644 --- a/ui-web/src/features/workspace/source.ts +++ b/ui-web/src/features/workspace/source.ts @@ -50,11 +50,20 @@ export function setShortener(fn: (p: string) => string): void { worse than plain text. Two cases are provable without a round trip: the text names the workspace, or Raven touched that file this session -- and since the viewer is no longer confined to the workspace, the second case now counts - wherever the file lives. Everything else stays plain text. */ + wherever the file lives. Everything else stays plain text. + + Both cases answer with the path as the author wrote it. The workspace case + used to answer with it made relative, from when the viewer's route rooted a + relative path at agent home; the route roots it at the SESSION's working + directory now (raven/rpc/methods/console.py's `_workspace_root`), so a + workspace path outside that directory was re-rooted onto it and served a 404 + -- the chip opened onto "that file is no longer there" for a file that was + there. Shortening is the panel's job either way (`shortPath` below), and the + other case here has always answered absolute. */ export const livePathOf = (s: string): string | null => { const text = String(s).trim().replace(/:\d+(?::\d+)?$/, '') if (!text || /\s/.test(text)) return null - if (/(?:^|\/)(?:\.raven\/)?workspace\/./.test(text)) return relToWorkspace(text) || text + if (/(?:^|\/)(?:\.raven\/)?workspace\/./.test(text)) return text const hit = workspaceChanges().find((c) => c.key === text || shorten(c.key) === text) return hit ? hit.key : null } diff --git a/ui-web/src/lib/prose.ts b/ui-web/src/lib/prose.ts index c7be59ef9..82ef6628b 100644 --- a/ui-web/src/lib/prose.ts +++ b/ui-web/src/lib/prose.ts @@ -19,9 +19,10 @@ export interface ProseTarget { } export interface ProseSource { - /* A workspace-relative path, or null when the string is not one that can be - opened. A path that merely looks like one must answer null: a dead link is - worse than no link. */ + /* The path a string names, as written, or null when it is not one that can + be opened. A path that merely looks like one must answer null: a dead link + is worse than no link -- and so is one that opens onto the wrong root, + which is why this is not shortened on the way through. */ pathOf(s: string): string | null /* What a markdown link's local target resolves to, or null. */ linkTargetOf(u: string): ProseTarget | null From 4f19708427ee09ac600a89df6098068878285880 Mon Sep 17 00:00:00 2001 From: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Date: Tue, 22 Sep 2026 01:33:40 +0800 Subject: [PATCH 3/4] fix(ui-web): the import row follows one run, not the page it was opened in Review follow-up on the previous commit. The marker that said "this page followed the run" was page-lifetime state, so a tab that followed one import and stayed open would draw the receipt of the next run somebody settled from the CLI: the flag was set once and never cleared, and any later settled status inherited it. It is per run now. A run in flight is `watching`; the read that finds it settled turns that into `followed`, the run's own signature, which is the same identity the dismissal already uses. A settled row is drawn when its click can still act on it or when it IS that run. A page outlives a run, so the marker cannot belong to the page. Co-authored-by: Claude (claude-opus-5[1m]) --- ui-web/src/app/install.test.ts | 7 ++- .../importSync/ImportSyncPage.test.tsx | 6 +- ui-web/src/features/importSync/store.test.ts | 33 ++++++++--- ui-web/src/features/importSync/store.ts | 55 +++++++++++++------ 4 files changed, 71 insertions(+), 30 deletions(-) diff --git a/ui-web/src/app/install.test.ts b/ui-web/src/app/install.test.ts index 07811d316..c3cd0101f 100644 --- a/ui-web/src/app/install.test.ts +++ b/ui-web/src/app/install.test.ts @@ -122,9 +122,12 @@ describe('the wizard asks for its import through the rail row (installSources)', wiring.installSources() setSources({ importSync: { + /* Settled with no request on file, so the row is drawn for one reason + only: this page followed the run. A resumable one would draw on its + own click and prove nothing. */ status: async () => ({ running: false, total: 1, submitted: 1, failed: 0, by_platform: {}, - phase: null, phases: null, tier: 'memory_files', platforms: ['hermes'], + phase: null, phases: null, tier: null, platforms: [], }), run: async (platforms: string[], tier: string) => { runs.push([platforms, tier]) @@ -137,7 +140,7 @@ describe('the wizard asks for its import through the rail row (installSources)', await ds('onboard').startImport(['hermes'], 'memory_files') expect(runs).toEqual([[['hermes'], 'memory_files']]) - expect(store.get().watched).toBe(true) + expect(store.view(store.get()).kind).toBe('done') store._resetForTests() }) }) diff --git a/ui-web/src/features/importSync/ImportSyncPage.test.tsx b/ui-web/src/features/importSync/ImportSyncPage.test.tsx index 795faf793..c9d5499fa 100644 --- a/ui-web/src/features/importSync/ImportSyncPage.test.tsx +++ b/ui-web/src/features/importSync/ImportSyncPage.test.tsx @@ -39,9 +39,9 @@ afterEach(() => { }) /* Drawn as a run this page followed, which is what every case below is about; - the one case about a run it did not follow turns `watched` off itself. */ + the one case about a run it did not follow clears `followed` itself. */ const draw = (st: ImportStatus | null, patch: Partial = {}): void => { - store.set({ status: st, watched: true, ...patch }) + store.set({ status: st, followed: st ? store.signature(st) : '', ...patch }) render() } const row = (): HTMLElement | null => document.querySelector('.importSync') @@ -118,7 +118,7 @@ describe('the import row', () => { /* An import somebody ran from the CLI weeks ago is still in the importer's file. It is not this reader's news and its count has no retry behind it. */ it('draws nothing for a finished run this page never followed', () => { - draw(status({ total: 2, submitted: 1, failed: 1, platforms: ['claude_code'] }), { watched: false }) + draw(status({ total: 2, submitted: 1, failed: 1, platforms: ['claude_code'] }), { followed: '' }) expect(row()).toBeNull() }) diff --git a/ui-web/src/features/importSync/store.test.ts b/ui-web/src/features/importSync/store.test.ts index 405d87f65..31f53e619 100644 --- a/ui-web/src/features/importSync/store.test.ts +++ b/ui-web/src/features/importSync/store.test.ts @@ -10,12 +10,13 @@ const status = (patch: Partial = {}): ImportStatus => ({ running: false, total: 0, submitted: 0, failed: 0, by_platform: {}, phase: null, phases: null, tier: null, platforms: [], ...patch, }) -/* `watched` defaults on: every case below is about a run this page asked for - or found in flight, which is the only kind that has ever drawn a settled - row. The cases that turn it off are the ones about a run left in the - importer's file by somebody else. */ +/* `followed` defaults to this very run: every case below is about a run this + page asked for or found in flight, which is the only kind that has ever + drawn a settled row. The cases that clear it are the ones about a run left + in the importer's file by somebody else. */ const state = (st: ImportStatus | null, patch: Partial = {}): store.ImportSyncState => ({ - status: st, starting: false, watched: true, dismissed: '', error: '', ...patch, + status: st, starting: false, watching: false, followed: st ? store.signature(st) : '', + dismissed: '', error: '', ...patch, }) interface Fake { @@ -283,12 +284,30 @@ describe('following a run', () => { await store.start(['hermes'], 'full') expect(store.view(store.get()).kind).toBe('done') }) + + /* A page outlives a run. Having followed one is not having followed the + next one somebody started from the CLI. */ + it('does not draw a later run it did not follow', async () => { + const mine = { tier: 'full' as const, platforms: ['hermes'] } + const f = fake([ + status({ running: true, total: 2, submitted: 1, ...mine }), + status({ total: 2, submitted: 2, ...mine, phases: { status: 'done', errors: [] } }), + /* Somebody else's run, settled in the same file while the tab stayed open. */ + status({ total: 5, submitted: 4, failed: 1, platforms: ['claude_code'] }), + ]) + setSources({ importSync: f.src }) + await store.refresh() + await store.refresh() + expect(store.view(store.get()).kind).toBe('done') + await store.refresh() + expect(store.view(store.get()).kind).toBe('hidden') + }) }) /* The row is the work outstanding, never a receipt: a run nobody here watched is on the rail only while its click can still do something. */ describe('a run left in the file by somebody else', () => { - const unwatched = { watched: false } + const unwatched = { followed: '' } it('is not drawn when it finished and nothing can be retried', () => { const cli = status({ total: 2, submitted: 2, platforms: ['claude_code', 'hermes'] }) @@ -299,7 +318,7 @@ describe('a run left in the file by somebody else', () => { has no retry behind it and the reader can do nothing but dismiss it. */ it('is not drawn when it finished with failures it cannot ask for again', () => { const cli = status({ total: 2, submitted: 1, failed: 1, platforms: ['claude_code', 'hermes'] }) - expect(store.view(state(cli, { watched: true }))) + expect(store.view(state(cli))) .toMatchObject({ kind: 'done', failed: 1, clickable: false }) expect(store.view(state(cli, unwatched)).kind).toBe('hidden') }) diff --git a/ui-web/src/features/importSync/store.ts b/ui-web/src/features/importSync/store.ts index b4e480df8..e8cd37f86 100644 --- a/ui-web/src/features/importSync/store.ts +++ b/ui-web/src/features/importSync/store.ts @@ -33,10 +33,14 @@ export interface ImportSyncState { /* Between asking for a run and hearing that it started: the scan the gateway does before it answers is the one stretch with no counts to show. */ starting: boolean - /* Whether this page has a run of its own to follow: it asked for one, or it - found one in flight. A settled run is only ever on the rail because of - this -- see `stale` below. */ - watched: boolean + /* A run of this page's own is in flight: it asked for one, or it found one + running. Becomes `followed` the moment that run settles. */ + watching: boolean + /* The run this page followed to its end, by the same signature `dismissed` + uses. A settled run is only ever on the rail because of this or because + its click still does something -- see `stale` below. Page state, never + stored: a reload has followed nothing. */ + followed: string dismissed: string error: string } @@ -58,13 +62,19 @@ export interface RowView { const HIDDEN: RowView = { kind: 'hidden', pct: 0, failed: 0, phase: null, source: null, clickable: false } /* Whether a run that is NOT running has earned a row. Two reasons it has: - something can still be done about it, or this page watched it happen. A run - that is neither is history -- the importer's file outlives every run that - wrote it, so a CLI import from weeks ago, or one an earlier reader started, - would otherwise greet the next reader as news. It is not news: nothing is - moving, the click does nothing, and the reader never asked for it. The row - is the work outstanding, never a receipt for work nobody here watched. */ -const stale = (s: ImportSyncState, clickable: boolean): boolean => !clickable && !s.watched + something can still be done about it, or this page followed THAT run to its + end. A run that is neither is history -- the importer's file outlives every + run that wrote it, so a CLI import from weeks ago, or one an earlier reader + started, would otherwise greet the next reader as news. It is not news: + nothing is moving, the click does nothing, and the reader never asked for + it. The row is the work outstanding, never a receipt for work nobody here + watched. + + Held per run rather than per page, because a page outlives a run: a tab that + followed one import and stayed open would otherwise draw the receipt of the + next one somebody ran from the CLI. */ +const stale = (s: ImportSyncState, st: ImportStatus, clickable: boolean): boolean => + !clickable && s.followed !== signature(st) const readDismissed = (): string => { try { return window.localStorage.getItem(DISMISSED_KEY) ?? '' } catch { return '' } @@ -74,7 +84,7 @@ const writeDismissed = (sig: string): void => { try { window.localStorage.setItem(DISMISSED_KEY, sig) } catch { /* a private window keeps nothing */ } } -const initial = (): ImportSyncState => ({ status: null, starting: false, watched: false, dismissed: readDismissed(), error: '' }) +const initial = (): ImportSyncState => ({ status: null, starting: false, watching: false, followed: '', dismissed: readDismissed(), error: '' }) const store = makeStore(initial()) @@ -119,19 +129,19 @@ export function view(s: ImportSyncState): RowView { if (!total && !phases) return HIDDEN /* Short of the total: the message pass was stopped, or the gateway lost it. */ if (settled < total) { - return stale(s, again) ? HIDDEN : { kind: 'paused', pct, failed: st.failed, phase: null, source: null, clickable: again } + return stale(s, st, again) ? HIDDEN : { kind: 'paused', pct, failed: st.failed, phase: null, source: null, clickable: again } } /* Settled, but the phases behind the pass never finished: no verdict on file for a run that recorded its request (lost before the phases began), or a verdict that says they were still running or were stopped. */ const unfinished = phases === null ? again : phases.status === 'pending' || phases.status === 'cancelled' if (unfinished) { - return stale(s, again) ? HIDDEN : { kind: 'paused', pct: 100, failed: st.failed, phase: null, source: null, clickable: again } + return stale(s, st, again) ? HIDDEN : { kind: 'paused', pct: 100, failed: st.failed, phase: null, source: null, clickable: again } } const failed = st.failed + (phases?.status === 'failed' ? phases.errors.length : 0) const retry = failed > 0 && again if (s.dismissed === signature(st)) return HIDDEN - if (stale(s, retry)) return HIDDEN + if (stale(s, st, retry)) return HIDDEN return { kind: 'done', pct: 100, failed, phase: null, source: null, clickable: retry } } @@ -151,7 +161,16 @@ function poll(on: boolean): void { export async function refresh(): Promise { try { const status = await source().status() - set({ status, error: '', ...(status.running ? { watched: true } : {}) }) + /* A run in flight is this page's to follow; the read that finds it settled + is where following turns into the one run whose row may be drawn. */ + store.set((prev) => ({ + ...prev, + status, + error: '', + ...(status.running + ? { watching: true } + : prev.watching ? { watching: false, followed: signature(status) } : {}), + })) poll(status.running) } catch (e) { set({ error: failure(e) }) @@ -165,7 +184,7 @@ export async function refresh(): Promise { makes the run this page's to follow, which is what keeps its finished row on the rail afterwards. */ export async function start(platforms: string[], tier: ImportTier): Promise { - set({ starting: true, watched: true, error: '' }) + set({ starting: true, watching: true, error: '' }) try { const r = await source().run(platforms, tier) /* Read after, then report: a refusal is this call's news, and the read @@ -206,5 +225,5 @@ export function dismiss(): void { export function _resetForTests(): void { poll(false) store._resetForTests() - store.set({ ...initial(), watched: false, dismissed: '' }) + store.set({ ...initial(), watching: false, followed: '', dismissed: '' }) } From d1a7ef1c262cfc0aa245bcae204872d3a8d3070b Mon Sep 17 00:00:00 2001 From: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Date: Tue, 22 Sep 2026 01:43:53 +0800 Subject: [PATCH 4/4] fix(ui-web): a refused import is not a run for the rail to follow Review follow-up. `start` marked the page as watching before `import.run` answered, so a refusal ("nothing to import") left the mark set and the read that follows adopted whatever settled run was already on file as this page's own. The wizard's sync step could then be exited into a receipt for an import somebody else ran. The mark is set only once the gateway says a run started. A refusal changes nothing, which is what a refusal means. Co-authored-by: Claude (claude-opus-5[1m]) --- ui-web/src/features/importSync/store.test.ts | 12 ++++++++++++ ui-web/src/features/importSync/store.ts | 10 +++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/ui-web/src/features/importSync/store.test.ts b/ui-web/src/features/importSync/store.test.ts index 31f53e619..9a406360c 100644 --- a/ui-web/src/features/importSync/store.test.ts +++ b/ui-web/src/features/importSync/store.test.ts @@ -285,6 +285,18 @@ describe('following a run', () => { expect(store.view(store.get()).kind).toBe('done') }) + /* A refusal is not a run. The gateway answers "nothing to import" without + starting anything, and whatever settled run is already on file is still + somebody else's. */ + it('follows nothing when the gateway refuses to start a run', async () => { + const f = fake([status({ total: 2, submitted: 2, platforms: ['claude_code'] })]) + f.src.run = async () => ({ started: false, total: 0, detail: 'nothing to import' }) + setSources({ importSync: f.src }) + const r = await store.start(['hermes'], 'full') + expect(r.started).toBe(false) + expect(store.view(store.get()).kind).toBe('hidden') + }) + /* A page outlives a run. Having followed one is not having followed the next one somebody started from the CLI. */ it('does not draw a later run it did not follow', async () => { diff --git a/ui-web/src/features/importSync/store.ts b/ui-web/src/features/importSync/store.ts index e8cd37f86..7b9ae42f1 100644 --- a/ui-web/src/features/importSync/store.ts +++ b/ui-web/src/features/importSync/store.ts @@ -180,13 +180,17 @@ export async function refresh(): Promise { /* Ask for a run and follow it. Every caller comes here -- the rail's own click and the wizard's sync step, which reaches it through the seam (src/app/ - install.ts) rather than calling the same method a second way. Asking is what - makes the run this page's to follow, which is what keeps its finished row on + install.ts) rather than calling the same method a second way. A run that + starts is the run this page follows, which is what keeps its finished row on the rail afterwards. */ export async function start(platforms: string[], tier: ImportTier): Promise { - set({ starting: true, watching: true, error: '' }) + set({ starting: true, error: '' }) try { const r = await source().run(platforms, tier) + /* Only a run that started is one to follow. A refusal ("nothing to + import") leaves whatever settled run is already on file untouched, and + the read below would otherwise adopt it as this page's. */ + if (r.started) set({ watching: true }) /* Read after, then report: a refusal is this call's news, and the read that follows it clears the transport error slot as every read does. */ await refresh()