mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-27 06:47:07 +00:00
fix(web): an agent's [[REF]] applied to an open tab is a link, not bracket text (BUG-3582) (#2052)
The designated applier handed the raw markdown to setContent, skipping the wiki-link → link conversion the load and the collab seed apply, so every open tab showed an agent write's [[TASK-5]] as literal text until reload. toEditorMarkdown (utils/markdown) is now the one conversion every path that hands the editor a body calls: the load's editorContent, the seed capture, the lazy seed, and the applier. rawCanonical's open-tab prediction (BUG-3540) mirrors the applier, so decideRawStale converts its texts the same way before the await; measured: a title link [[Fix it]] is now stored by an open tab as [[TASK-5]], and the unconverted prediction would have raised a false stale dialog. The stored body survives intact (the item's open question): [[TASK-5]] and [[TASK-5|shown text]] round-trip unchanged through the headless pipeline. e2e: an API PATCH while a tab is connected renders the link and stores the wiki-link. Red on main (no link), green 3/3. Identity gate: four rows re-reviewed (the two force-refresh continuations and the lazy seed moved only by their enclosing context; decideRawStale by the pre-await conversion). The shared $lib/utils/markdown import line is left unchanged (wikiLinksToMarkdown now unused there) because the gate hashes it into ~30 unrelated rows; toEditorMarkdown is on its own line. Claude-Session: https://claude.ai/code/session_01FAVXA5mJ5BNhKmv95jxQkQ
This commit is contained in:
xarmian
@@ -0,0 +1,45 @@
|
||||
import { test, expect } from './fixtures';
|
||||
import { browserLogin, EDITOR_SELECTOR, SYNCED_BADGE_SELECTOR, SYNC_TIMEOUT, seedDoc } from './lib/collab-helpers';
|
||||
import { authJson } from './lib/attachment-viewer';
|
||||
|
||||
/**
|
||||
* BUG-3582: an agent's body write that reaches an open tab through the
|
||||
* designated applier must show its `[[REF]]` as a link, as a fresh load does.
|
||||
* The applier handed the raw markdown to setContent, skipping the
|
||||
* wiki-link → link conversion the load and the seed apply, so every open
|
||||
* tab showed literal `[[TASK-5]]` bracket text until it was reloaded.
|
||||
*/
|
||||
test('an agent write through the applier shows its wiki-link as a link in the open tab', async ({ page, fixture, request }, testInfo) => {
|
||||
test.skip(testInfo.project.name !== 'desktop-chromium', 'one project is enough for a client-side conversion');
|
||||
test.setTimeout(90_000);
|
||||
const target = await seedDoc(fixture, request, 'B3582 link target');
|
||||
const host = await seedDoc(fixture, request, 'B3582 host');
|
||||
const t = (await (await request.get(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${target.slug}`, { headers: authJson(fixture) })).json()) as {
|
||||
collection_prefix: string;
|
||||
item_number: number;
|
||||
title: string;
|
||||
};
|
||||
const ref = `${t.collection_prefix}-${t.item_number}`;
|
||||
|
||||
await browserLogin(page);
|
||||
await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${host.slug}`);
|
||||
await expect(page.locator(SYNCED_BADGE_SELECTOR)).toBeVisible({ timeout: SYNC_TIMEOUT });
|
||||
|
||||
// The agent write: a plain API PATCH while the tab is connected, which the
|
||||
// server hands to the tab's applier.
|
||||
const patched = await request.patch(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${host.slug}`, {
|
||||
headers: authJson(fixture),
|
||||
data: { content: `See [[${ref}]] now.` },
|
||||
});
|
||||
expect(patched.ok(), await patched.text()).toBeTruthy();
|
||||
|
||||
const editor = page.locator(EDITOR_SELECTOR);
|
||||
await expect(editor).toContainText('See', { timeout: SYNC_TIMEOUT });
|
||||
await expect(editor.locator('a', { hasText: t.title }), 'the wiki-link renders as a link').toBeVisible();
|
||||
await expect(editor, 'no literal bracket text').not.toContainText('[[');
|
||||
|
||||
// The stored body keeps the wiki-link (the flush turns the link back).
|
||||
await expect
|
||||
.poll(async () => ((await (await request.get(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${host.slug}`, { headers: authJson(fixture) })).json()) as { content: string }).content, { timeout: 15_000 })
|
||||
.toContain(`[[${ref}]]`);
|
||||
});
|
||||
@@ -72,6 +72,7 @@
|
||||
import { goto } from '$app/navigation';
|
||||
import { beforeNavigate } from '$app/navigation';
|
||||
import { relativeTime, wikiLinksToMarkdown, markdownToWikiLinks, cleanBrokenLinks, unescapeDocLinks } from '$lib/utils/markdown';
|
||||
import { toEditorMarkdown } from '$lib/utils/markdown';
|
||||
import { canonicalEditorMarkdown } from '$lib/collab/canonicalMarkdown';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import { confirmDialog } from '$lib/stores/confirmDialog.svelte';
|
||||
@@ -775,12 +776,7 @@
|
||||
// Convert wiki-links to markdown links for the editor
|
||||
let editorContent = $derived.by(() => {
|
||||
if (!item) return '';
|
||||
const raw = item.content ?? '';
|
||||
const allItems = localIndex.getAll(wsSlug);
|
||||
if (allItems.length > 0 && raw.includes('[[')) {
|
||||
return wikiLinksToMarkdown(raw, allItems, wsSlug, username);
|
||||
}
|
||||
return raw;
|
||||
return toEditorMarkdown(item.content ?? '', localIndex.getAll(wsSlug), wsSlug, username);
|
||||
});
|
||||
|
||||
let contentDebounceTimer: ReturnType<typeof setTimeout> | undefined;
|
||||
@@ -2721,12 +2717,7 @@
|
||||
// regression than the bug this fixes. Mirrors the untracked
|
||||
// `baseline` read directly above for the same reason.
|
||||
const seedMd = untrack(() => {
|
||||
const allItemsAtLoad = localIndex.getAll(wsSlug);
|
||||
const raw =
|
||||
allItemsAtLoad.length > 0 && baseline.includes('[[')
|
||||
? wikiLinksToMarkdown(baseline, allItemsAtLoad, wsSlug, username)
|
||||
: baseline;
|
||||
return unescapeDocLinks(raw);
|
||||
return unescapeDocLinks(toEditorMarkdown(baseline, localIndex.getAll(wsSlug), wsSlug, username));
|
||||
});
|
||||
// The lazy-seed effect below OVERWRITES this with its own value
|
||||
// when (and only when) it actually calls setContent — its capture
|
||||
@@ -2815,7 +2806,9 @@
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
editorInstance.commands.setContent(markdown);
|
||||
// BUG-3582: the same conversion the load and the seed apply, or
|
||||
// an agent's [[REF]] lands as literal bracket text.
|
||||
editorInstance.commands.setContent(toEditorMarkdown(markdown, localIndex.getAll(wsSlug), wsSlug, username));
|
||||
// Brief notification so users see WHY their editor
|
||||
// just changed under them. The applier path is
|
||||
// triggered by external (CLI / MCP / API) writes
|
||||
@@ -3262,11 +3255,7 @@
|
||||
// wiki-links resolve to clickable refs. The 5s flush will
|
||||
// later round-trip back to canonical [[wiki-link]] form via
|
||||
// markdownToWikiLinks.
|
||||
const allItems = localIndex.getAll(wsSlug);
|
||||
const seedMd =
|
||||
allItems.length > 0 && seedRaw.includes('[[')
|
||||
? wikiLinksToMarkdown(seedRaw, allItems, wsSlug, username)
|
||||
: seedRaw;
|
||||
const seedMd = toEditorMarkdown(seedRaw, localIndex.getAll(wsSlug), wsSlug, username);
|
||||
|
||||
// Snapshot activeCollabContext now so the microtask below can
|
||||
// record the seed onto the SAME ctx object the flush path reads
|
||||
@@ -4884,7 +4873,12 @@
|
||||
// Our own texts, and the form an open rich tab stores for each (the
|
||||
// BUG-2995 round trip; rawCanonical.ts). Both compared EXACTLY.
|
||||
const own = rawBase.ownTexts();
|
||||
const forms = stored === '' || own.includes(stored) ? [] : await openTabForms(own, localIndex.getAll(wsSlug));
|
||||
// BUG-3582: an open tab's applier converts wiki-links before its
|
||||
// setContent, so the texts are converted the same way, here, before
|
||||
// the await, against this read's workspace and index.
|
||||
const linkIndex = localIndex.getAll(wsSlug);
|
||||
const ownForEditor = own.map((t) => toEditorMarkdown(t, linkIndex, wsSlug, username));
|
||||
const forms = stored === '' || own.includes(stored) ? [] : await openTabForms(ownForEditor, linkIndex);
|
||||
if (!item || item.id !== reqItemId || gen !== loadGeneration) return 'gone';
|
||||
if (!rawStaleRebased && isOwnBody(stored, [...own, ...forms])) {
|
||||
rawStaleRebased = true;
|
||||
|
||||
@@ -189,8 +189,12 @@ const ASYNC_FUNCTIONS: Record<string, Row> = {
|
||||
// moving the base or asking; after the dialog it checks again before the
|
||||
// reload writes. resolveRawStale resends only on a decided rebase/overwrite
|
||||
// (decideRawStale fenced it); its finally clears this pane's own flag.
|
||||
// BUG-3582: re-reviewed. Before the openTabForms await it now converts its
|
||||
// own texts with toEditorMarkdown (synchronous, pure, against this read's
|
||||
// workspace and index); nothing after the await reads live state anew, and
|
||||
// the fences after it are unchanged.
|
||||
decideRawStale: {
|
||||
reviewed: '119711a202f3',
|
||||
reviewed: '929baf56a305',
|
||||
why: 'item id and gen against loadGeneration after the read, after the open-tab canonical forms are computed, and after the dialog, before the base moves or the reload writes',
|
||||
},
|
||||
resolveRawStale: {
|
||||
@@ -271,8 +275,8 @@ const CONTINUATIONS: SignedRow[] = [
|
||||
why: 'connection state of this instance\'s own provider', reviewed: 'b0f9f4cc5f23',
|
||||
may: ['staleConnecting'],
|
||||
},
|
||||
{ call: /\.get\(refreshCtx\.wsSlug, refreshCtx\.itemId\) \.then\($/, body: /./, why: 'force-refresh fetch: refreshGen against loadGeneration. Re-reviewed for TASK-2199: the enclosing onForceRefresh first captures the edits it discards (offlineRecovery), synchronously, before any await. Re-reviewed for BUG-3526: the capture reads provider.editsMayBeMissing instead of provider.unsentLocalEdits, still synchronous and before the fetch; the continuation and its fences are unchanged', reviewed: 'ed881530cc3a' },
|
||||
{ call: /forceRefreshNonce \+= 1; \}\) \.catch\($/, body: /./, why: 'force-refresh failure: refreshGen against loadGeneration. Re-reviewed for TASK-2199 (the synchronous capture above it), and for BUG-3526 (only the input of that capture)', reviewed: '815a49a9adb8' },
|
||||
{ call: /\.get\(refreshCtx\.wsSlug, refreshCtx\.itemId\) \.then\($/, body: /./, why: 'force-refresh fetch: refreshGen against loadGeneration. Re-reviewed for TASK-2199: the enclosing onForceRefresh first captures the edits it discards (offlineRecovery), synchronously, before any await. Re-reviewed for BUG-3526: the capture reads provider.editsMayBeMissing instead of provider.unsentLocalEdits, still synchronous and before the fetch; the continuation and its fences are unchanged. Re-reviewed for BUG-3582: only the enclosing collab effect moved (onApplierRequest converts wiki-links before its setContent, synchronously, no await, no new write); this continuation is unchanged', reviewed: '5f2d4a91f1cf' },
|
||||
{ call: /forceRefreshNonce \+= 1; \}\) \.catch\($/, body: /./, why: 'force-refresh failure: refreshGen against loadGeneration. Re-reviewed for TASK-2199 (the synchronous capture above it), and for BUG-3526 (only the input of that capture). Re-reviewed for BUG-3582: only the enclosing collab effect moved (the synchronous wiki-link conversion in the applier); this continuation is unchanged', reviewed: 'de9c91d7f8df' },
|
||||
{
|
||||
call: /^setTimeout\($/,
|
||||
body: /teardownFlushed/,
|
||||
@@ -281,7 +285,7 @@ const CONTINUATIONS: SignedRow[] = [
|
||||
why: 're-arms the BUG-3005 teardown latch, itself identity-checked. Re-reviewed for TASK-2199: the prompt decision now also counts unsent collab edits and an uncopied offline version; nothing else moved. Re-reviewed for BUG-3556: the prompt reads the provider getter editsAtRiskOnClose instead of unsentLocalEdits (a read, no await, no write); nothing else moved. Re-reviewed for BUG-3571: one early return when a confirmed sign-out discard is under way (signOutDiscarding(), a module read; no await, no write); nothing else moved', reviewed: '4404c3f437e7',
|
||||
may: ['teardownFlushed'],
|
||||
},
|
||||
{ call: /^queueMicrotask\($/, body: /./, why: 'collab lazy seed: refuses a retired or re-identified context first', reviewed: 'ac93e315dd54' },
|
||||
{ call: /^queueMicrotask\($/, body: /./, why: 'collab lazy seed: refuses a retired or re-identified context first. Re-reviewed for BUG-3582: seedMd above it is now computed by toEditorMarkdown, the same value as the inline conversion it replaced; the microtask is unchanged', reviewed: '76af97b065fd' },
|
||||
{ call: /^tick\(\)\.then\($/, body: /./, why: 'schedules a focus frame; commits nothing itself', reviewed: 'acbf5de58fba' },
|
||||
{
|
||||
call: /^requestAnimationFrame\($/,
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { openTabForms } from './rawCanonical';
|
||||
import { toEditorMarkdown } from '$lib/utils/markdown';
|
||||
import type { Item } from '$lib/types';
|
||||
|
||||
// BUG-3540: the stored form an open rich tab writes for a raw save's text.
|
||||
describe('openTabForms', () => {
|
||||
@@ -16,4 +18,27 @@ describe('openTabForms', () => {
|
||||
const forms = await openTabForms(['a', '* b'], []);
|
||||
expect(forms).toEqual(['a', '- b']);
|
||||
});
|
||||
|
||||
// BUG-3582: the applier now converts a wiki-link to a link before
|
||||
// setContent, so an open tab stores the TITLE form `[[Fix it]]` as the
|
||||
// ref `[[TASK-5]]` (as the seed path always did). The prediction must
|
||||
// convert too, or a raw save through an open tab meets a false stale
|
||||
// dialog. Measured: without the conversion it predicts `[[Fix it]]`.
|
||||
it('predicts the ref form an open tab now stores for a title wiki-link', async () => {
|
||||
const index = [{ slug: 'fix-it', title: 'Fix it', item_number: 5, collection_prefix: 'TASK', collection_slug: 'tasks' }];
|
||||
const toEditor = (t: string) => toEditorMarkdown(t, index as unknown as Item[], 'docapp', 'dave');
|
||||
const [converted] = await openTabForms([toEditor('See [[Fix it]] now.')], index);
|
||||
const [unconverted] = await openTabForms(['See [[Fix it]] now.'], index);
|
||||
expect(converted).toBe('See [[TASK-5]] now.');
|
||||
expect(unconverted, 'control: the old prediction').toBe('See [[Fix it]] now.');
|
||||
});
|
||||
|
||||
it('a ref wiki-link round-trips unchanged (the stored body survives)', async () => {
|
||||
const index = [{ slug: 'fix-it', title: 'Fix it', item_number: 5, collection_prefix: 'TASK', collection_slug: 'tasks' }];
|
||||
const toEditor = (t: string) => toEditorMarkdown(t, index as unknown as Item[], 'docapp', 'dave');
|
||||
expect(await openTabForms(['See [[TASK-5]] now.', 'See [[TASK-5|shown text]] now.'].map(toEditor), index)).toEqual([
|
||||
'See [[TASK-5]] now.',
|
||||
'See [[TASK-5|shown text]] now.',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// The form an OPEN rich tab stores for markdown a raw save sent (BUG-3540).
|
||||
//
|
||||
// With a rich tab open, a raw save goes through that tab: the applier runs
|
||||
// `editor.commands.setContent(markdown)` and the tab's next collab flush stores
|
||||
// `editor.commands.setContent(toEditorMarkdown(markdown))` and the tab's next collab flush stores
|
||||
// `flushPipeline(getMarkdown(), localIndex)`. That is the BUG-2995 round trip,
|
||||
// which rewrites bullets, emphasis and blank lines, so the stored body is our
|
||||
// text in the editor's form, not byte for byte. Measured: '* one' lines beside
|
||||
@@ -33,6 +33,11 @@ function load() {
|
||||
}
|
||||
|
||||
/** The stored forms an open rich tab would write for each of `texts`. */
|
||||
/**
|
||||
* `texts` are what the applier hands setContent: already converted by
|
||||
* toEditorMarkdown (BUG-3582), since an open tab receives `[[REF]]` as a
|
||||
* link, not as text. The result is in the same order.
|
||||
*/
|
||||
export async function openTabForms(texts: readonly string[], index: readonly LinkEntry[]): Promise<string[]> {
|
||||
let loaded;
|
||||
try {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { createHeadlessEditor, flushPipeline } from '$lib/collab/materializer/entry';
|
||||
import { wikiLinksToMarkdown } from './markdown';
|
||||
import { wikiLinksToMarkdown, toEditorMarkdown } from './markdown';
|
||||
import type { Item } from '$lib/types';
|
||||
|
||||
// BUG-3315 U1: a wiki link whose text contains a character the markdown pipeline
|
||||
@@ -108,3 +108,17 @@ describe('BUG-3315 U1: link text with markdown/HTML-significant characters survi
|
||||
expect(flushed).toBe('see [[TASK-1|a > c]] here');
|
||||
});
|
||||
});
|
||||
|
||||
// BUG-3582: one conversion for every path that hands the editor a body.
|
||||
describe('toEditorMarkdown (BUG-3582)', () => {
|
||||
const items = [{ slug: 'fix-it', title: 'Fix it', item_number: 5, collection_prefix: 'TASK', collection_slug: 'tasks' }] as unknown as Parameters<typeof toEditorMarkdown>[1];
|
||||
it('turns [[REF]] into a link, as the seed always did', () => {
|
||||
const out = toEditorMarkdown('See [[TASK-5]] now.', items, 'docapp', 'dave');
|
||||
expect(out).not.toContain('[[');
|
||||
expect(out).toContain('](/dave/docapp/');
|
||||
});
|
||||
it('leaves a body with no wiki-link, or no index, untouched', () => {
|
||||
expect(toEditorMarkdown('plain', items, 'docapp', 'dave')).toBe('plain');
|
||||
expect(toEditorMarkdown('See [[TASK-5]]', [], 'docapp', 'dave')).toBe('See [[TASK-5]]');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -844,6 +844,19 @@ export function wikiLinksToMarkdown(content: string, items: Item[], workspaceSlu
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Stored markdown to the markdown the editor is given (BUG-3582): every
|
||||
* `[[REF]]` / `[[Title]]` becomes a link against the workspace's local item
|
||||
* index, exactly as `wikiLinksToMarkdown` does. ONE function for every path
|
||||
* that hands the editor a body, the load, the collab seed and the designated
|
||||
* applier, because the applier skipping it is how an agent's `[[TASK-5]]`
|
||||
* reached open tabs as literal bracket text. The flush's `markdownToWikiLinks`
|
||||
* turns the links back into wiki-links on the way out.
|
||||
*/
|
||||
export function toEditorMarkdown(stored: string, items: Item[], workspaceSlug: string, username?: string): string {
|
||||
return items.length > 0 && stored.includes('[[') ? wikiLinksToMarkdown(stored, items, workspaceSlug, username) : stored;
|
||||
}
|
||||
|
||||
/**
|
||||
* Convert markdown links back to wiki-link storage syntax.
|
||||
* When the link's URL resolves to an item with a ref, emit [[REF]] (or
|
||||
|
||||
Reference in New Issue
Block a user