From f6b55660efa3b596bfe5a8dce773a72a0a793827 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Thu, 13 Aug 2026 12:46:29 -0400 Subject: [PATCH 1/6] Stop restarting a code diff that is already loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A code block's arguments are recomputed on every invalidation of the room resource, which during streaming arrives continuously. `modify` treated each one as a reason to start over: it cleared any error and performed the load again, and because that load is a restartable task, performing it cancelled the request in flight. Two things followed. The resource sat holding no code and no error, which is the one state the template renders as nothing at all — a code block with a header, an editor and an apply button in the DOM but no content in any of them. And since cancelling the task does not cancel the request behind it, every restart left its fetch running and started another, so a patch creating a new file — where a 404 is the expected answer — asked the realm for that file thousands of times. The requests then contended with each other, so each load took longer, so more of them were cancelled before they could finish. Restart only when the file or the patch actually changes; otherwise leave a load that is already under way alone. Give the load an abort signal so a superseded one stops rather than merely having its answer discarded, and thread that signal through getSource. Keep the in-flight flag untracked, since consuming tracked state in `modify` would re-enter it on every change. Co-Authored-By: Claude Opus 5 (1M context) --- .../ai-assistant/message/aibot-message.gts | 36 +++++++- packages/host/app/resources/code-diff.ts | 89 ++++++++++++++----- packages/host/app/services/card-service.ts | 6 +- .../formatted-aibot-message-test.gts | 83 +++++++++++++++++ 4 files changed, 192 insertions(+), 22 deletions(-) diff --git a/packages/host/app/components/ai-assistant/message/aibot-message.gts b/packages/host/app/components/ai-assistant/message/aibot-message.gts index a916b863085..18932a92811 100644 --- a/packages/host/app/components/ai-assistant/message/aibot-message.gts +++ b/packages/host/app/components/ai-assistant/message/aibot-message.gts @@ -6,7 +6,7 @@ import { htmlSafe } from '@ember/template'; import Component from '@glimmer/component'; import { cached, tracked } from '@glimmer/tracking'; -import { Alert } from '@cardstack/boxel-ui/components'; +import { Alert, LoadingIndicator } from '@cardstack/boxel-ui/components'; import { and, bool, eq } from '@cardstack/boxel-ui/helpers'; import { markdownToHtml } from '@cardstack/runtime-common/marked-sync'; @@ -343,6 +343,27 @@ class HtmlGroupCodeBlock extends Component { @modifiedCode={{this.codeDiffResource.modifiedCode}} /> + {{else}} + {{! The header needs only the file URL, which is known as soon as + the patch is parsed, so a diff still being fetched can name the file + it belongs to. Without this the whole block renders empty and an + unresolved patch is indistinguishable from nothing at all. }} + {{#if @codeData.fileUrl}} + + {{/if}} + {{#if this.codeDiffResource.isLoadingDiff}} +
+ + Loading diff… +
+ {{/if}} {{/if}} {{#if this.codePatchErrorMessage}} @@ -374,5 +395,18 @@ class HtmlGroupCodeBlock extends Component { {{/if}} + + } diff --git a/packages/host/app/resources/code-diff.ts b/packages/host/app/resources/code-diff.ts index d99208b780b..b7adbfc218b 100644 --- a/packages/host/app/resources/code-diff.ts +++ b/packages/host/app/resources/code-diff.ts @@ -1,3 +1,4 @@ +import { registerDestructor } from '@ember/destroyable'; import { service } from '@ember/service'; import { tracked } from '@glimmer/tracking'; @@ -27,48 +28,63 @@ export class CodeDiffResource extends Resource { @tracked errorMessage: string | undefined | null = null; codePatchStatus: CodePatchStatus | undefined | null = null; + // Deliberately untracked: `modify` consults this to decide whether a load is + // already under way, and consuming tracked state there would re-enter + // `modify` every time the load changed it. The template reads the task's own + // `isRunning` instead, which is tracked and safe to render from. + private loadInFlight = false; + private abortController: AbortController | undefined; + @service declare private cardService: CardService; @service declare private toolService: ToolService; + constructor(owner: object) { + super(owner); + registerDestructor(this, () => this.abortController?.abort()); + } + modify(_positional: never[], named: CodeDiffResourceArgs['named']) { let { fileUrl, searchReplaceBlock, codePatchStatus } = named; let fileOrPatchChanged = this.fileUrl !== fileUrl || this.searchReplaceBlock !== searchReplaceBlock; let appliedStateChanged = - this.codePatchStatus === 'applied' || codePatchStatus === 'applied'; - if (fileOrPatchChanged || appliedStateChanged) { - this.originalCode = null; - this.modifiedCode = null; - } - this.errorMessage = null; + this.codePatchStatus !== codePatchStatus && + (this.codePatchStatus === 'applied' || codePatchStatus === 'applied'); + let inputsChanged = fileOrPatchChanged || appliedStateChanged; + this.fileUrl = fileUrl; this.searchReplaceBlock = searchReplaceBlock; this.codePatchStatus = codePatchStatus; - if (!fileUrl) { + // These arguments are recomputed on every invalidation of the room + // resource, which during streaming arrives continuously — so `modify` runs + // far more often than anything about this diff actually changes. Only a + // change to what is being diffed justifies discarding what we have and + // starting again. Restarting on every invalidation cancelled the load + // mid-flight and cleared the error along with it, leaving the resource + // holding neither code nor a message to show, and asking the realm for the + // file again each time it happened. + if (!inputsChanged) { + if (this.isDataLoaded || this.errorMessage || this.loadInFlight) { + return; + } + } else { this.originalCode = null; this.modifiedCode = null; + this.errorMessage = null; + } + + if (!fileUrl) { this.errorMessage = 'Missing file URL in the code block'; return; } if (!searchReplaceBlock) { - this.originalCode = null; - this.modifiedCode = null; this.errorMessage = 'Missing search and replace block'; return; } - if ( - !fileOrPatchChanged && - codePatchStatus !== 'applied' && - this.originalCode != null && - this.modifiedCode != null - ) { - return; - } - this.load.perform(); } @@ -76,7 +92,33 @@ export class CodeDiffResource extends Resource { return this.originalCode != null && this.modifiedCode != null; } + // True between starting a load and having something to show for it, so a + // patch whose diff has not arrived can say so rather than render as nothing. + get isLoadingDiff() { + return this.load.isRunning && !this.isDataLoaded && !this.errorMessage; + } + private load = restartableTask(async () => { + // Cancelling the task does not cancel a request already in flight, so + // without this an abandoned load runs to completion and its answer is + // merely discarded — the work still reaches the realm. + this.abortController?.abort(); + let abortController = new AbortController(); + this.abortController = abortController; + this.loadInFlight = true; + try { + await this.loadDiff(abortController.signal); + } finally { + // Only the newest run owns the flag. A superseded run settling later must + // not report that loading has finished on behalf of the one that replaced + // it. + if (this.abortController === abortController) { + this.loadInFlight = false; + } + } + }); + + private async loadDiff(signal: AbortSignal) { let { fileUrl, searchReplaceBlock, codePatchStatus } = this; if (codePatchStatus === 'applied') { this.originalCode = null; @@ -96,13 +138,20 @@ export class CodeDiffResource extends Resource { return; } try { - let result = await this.cardService.getSource(new URL(fileUrl)); + let result = await this.cardService.getSource(new URL(fileUrl), { + signal, + }); if (result.status === 404) { this.originalCode = ''; // We are creating a new file, so we don't have the original code } else { this.originalCode = result.content; } } catch (error) { + if (signal.aborted) { + // Superseded by a newer load, or the resource went away. Neither is a + // failure to report. + return; + } this.errorMessage = `Failed to load code from ${fileUrl}`; return; } @@ -123,7 +172,7 @@ export class CodeDiffResource extends Resource { this.errorMessage = error instanceof Error ? error.message : String(error); } - }); + } } export function getCodeDiffResultResource( diff --git a/packages/host/app/services/card-service.ts b/packages/host/app/services/card-service.ts index 86d05550240..620d7e4116e 100644 --- a/packages/host/app/services/card-service.ts +++ b/packages/host/app/services/card-service.ts @@ -247,11 +247,15 @@ export default class CardService extends Service { return serialized; } - async getSource(url: RealmResourceIdentifier | URL) { + async getSource( + url: RealmResourceIdentifier | URL, + opts?: { signal?: AbortSignal }, + ) { let response = await this.network.authedFetch(url, { headers: { Accept: 'application/vnd.card+source', }, + signal: opts?.signal, }); return { status: response.status, diff --git a/packages/host/tests/integration/components/formatted-aibot-message-test.gts b/packages/host/tests/integration/components/formatted-aibot-message-test.gts index 6643109cb92..db17667405c 100644 --- a/packages/host/tests/integration/components/formatted-aibot-message-test.gts +++ b/packages/host/tests/integration/components/formatted-aibot-message-test.gts @@ -370,6 +370,89 @@ let c = 3; ); }); + // The code block's arguments are recomputed on every invalidation of the room + // resource, which during streaming arrives continuously. Each of those used to + // restart the load, cancelling the request in flight and leaving the block + // with neither a diff nor an error to show — an empty box — while asking the + // realm for the file again every time. + test('re-rendering with unchanged inputs neither refetches the file nor blanks the diff', async function (assert) { + let getSourceCallCount = 0; + cardService.getSource = async () => { + getSourceCallCount++; + return Promise.resolve({ + status: 200, + contentType: 'application/vnd.card+source', + content: 'let a = 1;\nlet b = 2;', + }); + }; + + let monacoSDK = await monacoService.getMonacoContext(); + let component: any = null; + + class TestComponent extends Component { + @tracked htmlParts = []; + + constructor(owner: Owner, args: any) { + super(owner, args); + component = this; + } + + + } + + await renderComponent(TestComponent); + if (!component) { + throw new Error('Component not found'); + } + + let codeBlockHtml = `
+https://example.com/file.ts
+${SEARCH_MARKER}
+let a = 1;
+${SEPARATOR_MARKER}
+let a = 2;
+${REPLACE_MARKER}
+
`; + + component.htmlParts = parseHtmlContent(codeBlockHtml, roomId, eventId); + await settled(); + await waitFor('.code-block-diff'); + assert.strictEqual( + getSourceCallCount, + 1, + 'the file is fetched once to build the diff', + ); + + // Fresh CodeData objects carrying identical values, which is what an + // invalidation of the room resource produces. + for (let i = 0; i < 5; i++) { + component.htmlParts = parseHtmlContent(codeBlockHtml, roomId, eventId); + await settled(); + } + + assert.strictEqual( + getSourceCallCount, + 1, + 'unchanged inputs do not send the realm another request', + ); + assert.dom('.code-block-diff').exists('the diff survives re-rendering'); + assert + .dom('[data-test-apply-code-button]') + .exists('the apply button survives re-rendering'); + assert + .dom('[data-test-code-patch-loading]') + .doesNotExist('the block is not left in a loading state'); + }); + test('it will render either standard code editor or diff editor during streaming depending on whether the individual search/replace blocks are complete', async function (assert) { let monacoSDK = await monacoService.getMonacoContext(); let component: any = null; From 283ba0e3c8813ee6aee7939ef428550c753eb002 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Thu, 13 Aug 2026 16:01:31 -0400 Subject: [PATCH 2/6] Show the pending header only while a diff is loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rendering it for any truthy file URL reached the header with values that are not URLs — the model names the file it is patching, and what it writes is not always parseable. `fileName` constructs a URL from it unguarded, so the getter threw and took the whole message render down with it. A patch that failed does not need the header anyway: the footer's alert already speaks for it, and the empty block this was meant to fix is the pending one. Restrict the header to that case, and stop `fileName` from throwing so a header that is merely wrong cannot break the render. The fallback matters beyond this branch — the standard code block renders the same header for any truthy file url. Co-Authored-By: Claude Opus 5 (1M context) --- .../code-block/diff-editor-header.gts | 14 +++++++++++++- .../ai-assistant/message/aibot-message.gts | 16 ++++++++-------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/host/app/components/ai-assistant/code-block/diff-editor-header.gts b/packages/host/app/components/ai-assistant/code-block/diff-editor-header.gts index 0e69ebdaa6c..e97dc033fd8 100644 --- a/packages/host/app/components/ai-assistant/code-block/diff-editor-header.gts +++ b/packages/host/app/components/ai-assistant/code-block/diff-editor-header.gts @@ -191,7 +191,19 @@ export default class CodeBlockDiffEditorHeader extends Component { @modifiedCode={{this.codeDiffResource.modifiedCode}} /> - {{else}} + {{else if this.codeDiffResource.isLoadingDiff}} {{! The header needs only the file URL, which is known as soon as the patch is parsed, so a diff still being fetched can name the file it belongs to. Without this the whole block renders empty and an - unresolved patch is indistinguishable from nothing at all. }} + unresolved patch is indistinguishable from nothing at all. A patch + that failed is not shown here — it has the footer's alert to speak + for it. }} {{#if @codeData.fileUrl}} { @codePatchErrorMessage={{this.codePatchErrorMessage}} /> {{/if}} - {{#if this.codeDiffResource.isLoadingDiff}} -
- - Loading diff… -
- {{/if}} +
+ + Loading diff… +
{{/if}} {{#if this.codePatchErrorMessage}} From fe6879a19fbdf328200bebd0640d3760404024d4 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 17 Aug 2026 12:17:33 -0400 Subject: [PATCH 3/6] Cover the parts of the diff resource that had nothing holding them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three behaviours shipped without tests, each one the kind that breaks quietly. A superseded load has to abort its request, since cancelling the task leaves the fetch running and only discards the answer — the difference between a bounded restart and a request storm is invisible from the rendered output. A patch whose diff has not arrived has to say so. Asserting only that the loading state is absent once settled would pass just as well if it never rendered at all, which is the bug this was written to prevent. And a file name that is not a URL has to reach the header without throwing. The model names the file it is patching and what it writes is not always parseable; constructing a URL from it unguarded took down the whole message. Co-Authored-By: Claude Opus 5 (1M context) --- .../formatted-aibot-message-test.gts | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) diff --git a/packages/host/tests/integration/components/formatted-aibot-message-test.gts b/packages/host/tests/integration/components/formatted-aibot-message-test.gts index db17667405c..946039451af 100644 --- a/packages/host/tests/integration/components/formatted-aibot-message-test.gts +++ b/packages/host/tests/integration/components/formatted-aibot-message-test.gts @@ -453,6 +453,166 @@ ${REPLACE_MARKER} .doesNotExist('the block is not left in a loading state'); }); + // Cancelling the task does not cancel the request behind it, so a superseded + // load has to be stopped explicitly or its work still reaches the realm. + test('a superseded load aborts its request rather than leaving it running', async function (assert) { + let signals: (AbortSignal | undefined)[] = []; + cardService.getSource = async ( + _url: any, + opts?: { signal?: AbortSignal }, + ) => { + signals.push(opts?.signal); + return Promise.resolve({ + status: 200, + contentType: 'application/vnd.card+source', + content: 'let a = 1;\nlet b = 2;', + }); + }; + + let monacoSDK = await monacoService.getMonacoContext(); + let component: any = null; + + class TestComponent extends Component { + @tracked htmlParts = []; + + constructor(owner: Owner, args: any) { + super(owner, args); + component = this; + } + + + } + + await renderComponent(TestComponent); + if (!component) { + throw new Error('Component not found'); + } + + let blockFor = ( + replacement: string, + ) => `
+https://example.com/file.ts
+${SEARCH_MARKER}
+let a = 1;
+${SEPARATOR_MARKER}
+${replacement}
+${REPLACE_MARKER}
+
`; + + component.htmlParts = parseHtmlContent( + blockFor('let a = 2;'), + roomId, + eventId, + ); + await settled(); + assert.strictEqual(signals.length, 1, 'the first load ran'); + assert.false( + signals[0]?.aborted, + 'its signal is live while it is the current load', + ); + + // A genuinely different patch, so the load is restarted rather than reused. + component.htmlParts = parseHtmlContent( + blockFor('let a = 3;'), + roomId, + eventId, + ); + await settled(); + + assert.strictEqual(signals.length, 2, 'the changed patch started a load'); + assert.true( + signals[0]?.aborted, + 'the superseded load had its request aborted', + ); + assert.false(signals[1]?.aborted, 'the current load is untouched'); + }); + + test('a patch whose diff has not arrived says so instead of rendering nothing', async function (assert) { + let releaseGetSource: (() => void) | undefined; + cardService.getSource = async () => { + await new Promise((resolve) => { + releaseGetSource = resolve; + }); + return { + status: 200, + contentType: 'application/vnd.card+source', + content: 'let a = 1;\nlet b = 2;', + }; + }; + + // Deliberately not awaited: the assertion is about the window before the + // fetch settles, which is exactly what `settled()` would wait past. + let rendering = renderFormattedAiBotMessage({ + htmlParts: parseHtmlContent( + `
+https://example.com/file.ts
+${SEARCH_MARKER}
+let a = 1;
+${SEPARATOR_MARKER}
+let a = 2;
+${REPLACE_MARKER}
+
`, + roomId, + eventId, + ), + isStreaming: false, + isLastAssistantMessage: true, + }); + + try { + await waitFor('[data-test-code-patch-loading]'); + assert + .dom('[data-test-code-patch-loading]') + .exists('the pending patch is visible while its diff loads'); + assert + .dom('[data-test-file-name]') + .containsText('file.ts', 'and it names the file it belongs to'); + } finally { + // Release even if the wait above failed, so a broken assertion fails the + // test rather than leaving the render promise pending forever. + releaseGetSource?.(); + await rendering; + } + + assert + .dom('[data-test-code-patch-loading]') + .doesNotExist('the loading state gives way to the diff'); + assert.dom('.code-block-diff').exists(); + }); + + // The model names the file it is patching and what it writes is not always a + // URL. Constructing one unguarded threw out of a getter, taking the whole + // message render down rather than just the header. + test('a file name that is not a URL does not break the render', async function (assert) { + await renderFormattedAiBotMessage({ + htmlParts: parseHtmlContent( + `
+malformed file url
+${SEARCH_MARKER}
+let a = 1;
+
`, + roomId, + eventId, + ), + isStreaming: false, + isLastAssistantMessage: true, + }); + + assert.dom('.code-block').exists('the message still renders'); + assert + .dom('[data-test-file-name]') + .containsText('malformed file url', 'falling back to the raw name'); + }); + test('it will render either standard code editor or diff editor during streaming depending on whether the individual search/replace blocks are complete', async function (assert) { let monacoSDK = await monacoService.getMonacoContext(); let component: any = null; From f3d3f89b3e7ca6550dd9764bba01643d1b8f096c Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 17 Aug 2026 13:47:00 -0400 Subject: [PATCH 4/6] Check ownership after every await in the diff load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The load runs outside the task, so cancelling the task stops it only at the await inside the task body. Past that point the work carries on regardless, and the abort signal reaches the fetch and nothing else — so a load superseded while awaiting the patch tool resumed later and wrote its answer over the state belonging to the patch that replaced it, putting a stale diff on screen and mutating a resource that may already be gone. Check the signal after each await before writing anything, and apply the patch against the code this load fetched rather than whatever the resource holds by the time it resolves. Co-Authored-By: Claude Opus 5 (1M context) --- packages/host/app/resources/code-diff.ts | 27 ++++-- .../formatted-aibot-message-test.gts | 97 +++++++++++++++++++ 2 files changed, 118 insertions(+), 6 deletions(-) diff --git a/packages/host/app/resources/code-diff.ts b/packages/host/app/resources/code-diff.ts index b7adbfc218b..49c39c0713c 100644 --- a/packages/host/app/resources/code-diff.ts +++ b/packages/host/app/resources/code-diff.ts @@ -137,15 +137,22 @@ export class CodeDiffResource extends Resource { if (!fileUrl || !searchReplaceBlock) { return; } + // This runs outside the task, so cancelling the task does not stop it here: + // every await is a point where a newer load may have taken over, and the + // abort signal only reaches the fetch. Check ownership after each one + // before writing to the resource, or a superseded load resumes later and + // overwrites the state — and the diff on screen — belonging to the patch + // that replaced it. + let originalCode: string; try { let result = await this.cardService.getSource(new URL(fileUrl), { signal, }); - if (result.status === 404) { - this.originalCode = ''; // We are creating a new file, so we don't have the original code - } else { - this.originalCode = result.content; + if (signal.aborted) { + return; } + // A 404 means we are creating a new file, so there is no original code. + originalCode = result.status === 404 ? '' : result.content; } catch (error) { if (signal.aborted) { // Superseded by a newer load, or the resource went away. Neither is a @@ -155,6 +162,7 @@ export class CodeDiffResource extends Resource { this.errorMessage = `Failed to load code from ${fileUrl}`; return; } + this.originalCode = originalCode; let applySearchReplaceBlockCommand = new ApplySearchReplaceBlockTool( this.toolService.toolContext, @@ -163,12 +171,19 @@ export class CodeDiffResource extends Resource { try { let { resultContent: patchedCode } = await applySearchReplaceBlockCommand.execute({ - fileContent: this.originalCode, + // This load's own code, not whatever the resource holds by now. + fileContent: originalCode, codeBlock: searchReplaceBlock, }); + if (signal.aborted) { + return; + } this.modifiedCode = patchedCode; } catch (error) { - this.modifiedCode = this.originalCode; + if (signal.aborted) { + return; + } + this.modifiedCode = originalCode; this.errorMessage = error instanceof Error ? error.message : String(error); } diff --git a/packages/host/tests/integration/components/formatted-aibot-message-test.gts b/packages/host/tests/integration/components/formatted-aibot-message-test.gts index 946039451af..9ec23b9744e 100644 --- a/packages/host/tests/integration/components/formatted-aibot-message-test.gts +++ b/packages/host/tests/integration/components/formatted-aibot-message-test.gts @@ -536,6 +536,103 @@ ${REPLACE_MARKER} assert.false(signals[1]?.aborted, 'the current load is untouched'); }); + // `loadDiff` runs outside the task, so cancelling the task does not stop it — + // it keeps going at every await, and the abort signal only reaches the fetch. + // A superseded load that resumes afterwards must not write its answer over + // the patch that replaced it. + test('a superseded load that resolves late does not overwrite the newer diff', async function (assert) { + let releases: Array<(content: string) => void> = []; + cardService.getSource = async () => { + let content = await new Promise((resolve) => { + releases.push(resolve); + }); + return { + status: 200, + contentType: 'application/vnd.card+source', + content, + }; + }; + + let monacoSDK = await monacoService.getMonacoContext(); + let component: any = null; + + class TestComponent extends Component { + @tracked htmlParts = []; + + constructor(owner: Owner, args: any) { + super(owner, args); + component = this; + } + + + } + + await renderComponent(TestComponent); + if (!component) { + throw new Error('Component not found'); + } + + let blockFor = ( + replacement: string, + ) => `
+https://example.com/file.ts
+${SEARCH_MARKER}
+let a = 1;
+${SEPARATOR_MARKER}
+${replacement}
+${REPLACE_MARKER}
+
`; + + // The superseded load, left mid-flight. + component.htmlParts = parseHtmlContent( + blockFor('let a = 2;'), + roomId, + eventId, + ); + await waitUntil(() => releases.length === 1); + + // A different patch takes over before the first one has its source. + component.htmlParts = parseHtmlContent( + blockFor('let a = 3;'), + roomId, + eventId, + ); + await waitUntil(() => releases.length === 2); + + // The newer load finishes first and puts its diff on screen. + releases[1]('let a = 1;'); + await settled(); + await waitFor('.code-block-diff'); + await waitUntil(() => + ( + document.getElementsByClassName('view-lines')[2] as HTMLElement + )?.innerText.includes('let a = 3;'), + ); + + // Only now does the abandoned one come back with its answer. + releases[0]('let a = 1;'); + await settled(); + + assert + .dom('[data-test-apply-code-button]') + .exists('the newer patch is still the one on offer'); + assert.ok( + ( + document.getElementsByClassName('view-lines')[2] as HTMLElement + )?.innerText.includes('let a = 3;'), + 'the diff still shows the newer replacement, not the abandoned one', + ); + }); + test('a patch whose diff has not arrived says so instead of rendering nothing', async function (assert) { let releaseGetSource: (() => void) | undefined; cardService.getSource = async () => { From 38155d61dd06c8c3ba4fe8945f66850e49e7fb65 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 17 Aug 2026 14:42:14 -0400 Subject: [PATCH 5/6] Ask which pane shows the replacement rather than guessing an index The assertion read `view-lines[2]`, copied from a test that renders several editors. This one renders a single diff, so that index held nothing, the predicate was never true, and the wait ran to its timeout instead of failing on anything real. Search every pane for the replacement instead, and wait on the insert decoration the way the neighbouring diff test already does. The abandoned patch's replacement is now asserted absent too, which is the property the test exists to hold. Co-Authored-By: Claude Opus 5 (1M context) --- .../formatted-aibot-message-test.gts | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/packages/host/tests/integration/components/formatted-aibot-message-test.gts b/packages/host/tests/integration/components/formatted-aibot-message-test.gts index 9ec23b9744e..06fdb4a4f06 100644 --- a/packages/host/tests/integration/components/formatted-aibot-message-test.gts +++ b/packages/host/tests/integration/components/formatted-aibot-message-test.gts @@ -608,15 +608,23 @@ ${REPLACE_MARKER} ); await waitUntil(() => releases.length === 2); + // Which pane holds the replacement depends on how many editors are on the + // page, so ask whether any of them shows it rather than picking an index. + let shownInDiff = (text: string) => + Array.from(document.getElementsByClassName('view-lines')).some((el) => + (el as HTMLElement).innerText.includes(text), + ); + // The newer load finishes first and puts its diff on screen. releases[1]('let a = 1;'); await settled(); - await waitFor('.code-block-diff'); - await waitUntil(() => - ( - document.getElementsByClassName('view-lines')[2] as HTMLElement - )?.innerText.includes('let a = 3;'), + await waitUntil( + () => + document.querySelectorAll('.code-block-diff .cdr.line-insert').length > + 0, + { timeout: 5000 }, ); + await waitUntil(() => shownInDiff('let a = 3;'), { timeout: 5000 }); // Only now does the abandoned one come back with its answer. releases[0]('let a = 1;'); @@ -625,11 +633,13 @@ ${REPLACE_MARKER} assert .dom('[data-test-apply-code-button]') .exists('the newer patch is still the one on offer'); - assert.ok( - ( - document.getElementsByClassName('view-lines')[2] as HTMLElement - )?.innerText.includes('let a = 3;'), - 'the diff still shows the newer replacement, not the abandoned one', + assert.true( + shownInDiff('let a = 3;'), + 'the diff still shows the newer replacement', + ); + assert.false( + shownInDiff('let a = 2;'), + 'the abandoned patch never reaches the screen', ); }); From 46d5fc23b714913e81da20a909ab900d3b73506f Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 17 Aug 2026 16:05:19 -0400 Subject: [PATCH 6/6] Observe the clobber through an error, not the diff editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading the replacement back out of monaco made the test depend on how the diff editor lays out its panes and which decorations it emits for a one-line change — twice that guess was wrong, and both times it surfaced as a wait running to its timeout rather than as anything about the behaviour under test. Give the abandoned load a search string the file does not contain, so finishing is something it cannot do quietly: it reports that the patch would not apply. The property then reads directly off the absence of that error, and the only DOM this test touches is the diff container and the alert. Co-Authored-By: Claude Opus 5 (1M context) --- .../formatted-aibot-message-test.gts | 48 ++++++++----------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/packages/host/tests/integration/components/formatted-aibot-message-test.gts b/packages/host/tests/integration/components/formatted-aibot-message-test.gts index 06fdb4a4f06..9d702ea2a65 100644 --- a/packages/host/tests/integration/components/formatted-aibot-message-test.gts +++ b/packages/host/tests/integration/components/formatted-aibot-message-test.gts @@ -582,65 +582,57 @@ ${REPLACE_MARKER} } let blockFor = ( + search: string, replacement: string, ) => `
 https://example.com/file.ts
 ${SEARCH_MARKER}
-let a = 1;
+${search}
 ${SEPARATOR_MARKER}
 ${replacement}
 ${REPLACE_MARKER}
`; - // The superseded load, left mid-flight. + // The abandoned load searches for something the file does not contain, so + // if it ever finishes it reports that the patch could not be applied. That + // makes the clobber observable without reading anything out of the diff + // editor, whose internals are not this test's business. component.htmlParts = parseHtmlContent( - blockFor('let a = 2;'), + blockFor('let missing = 0;', 'let a = 2;'), roomId, eventId, ); await waitUntil(() => releases.length === 1); - // A different patch takes over before the first one has its source. + // A patch that does apply takes over before the first one has its source. component.htmlParts = parseHtmlContent( - blockFor('let a = 3;'), + blockFor('let a = 1;', 'let a = 3;'), roomId, eventId, ); await waitUntil(() => releases.length === 2); - // Which pane holds the replacement depends on how many editors are on the - // page, so ask whether any of them shows it rather than picking an index. - let shownInDiff = (text: string) => - Array.from(document.getElementsByClassName('view-lines')).some((el) => - (el as HTMLElement).innerText.includes(text), - ); - // The newer load finishes first and puts its diff on screen. releases[1]('let a = 1;'); await settled(); - await waitUntil( - () => - document.querySelectorAll('.code-block-diff .cdr.line-insert').length > - 0, - { timeout: 5000 }, - ); - await waitUntil(() => shownInDiff('let a = 3;'), { timeout: 5000 }); + await waitFor('.code-block-diff'); + assert + .dom('[data-test-error-message]') + .doesNotExist('the applicable patch loaded cleanly'); // Only now does the abandoned one come back with its answer. releases[0]('let a = 1;'); await settled(); + assert + .dom('[data-test-error-message]') + .doesNotExist( + 'the abandoned load does not report its failure over the newer patch', + ); + assert.dom('.code-block-diff').exists('the newer diff is still on screen'); assert .dom('[data-test-apply-code-button]') - .exists('the newer patch is still the one on offer'); - assert.true( - shownInDiff('let a = 3;'), - 'the diff still shows the newer replacement', - ); - assert.false( - shownInDiff('let a = 2;'), - 'the abandoned patch never reaches the screen', - ); + .exists('and is still the one on offer'); }); test('a patch whose diff has not arrived says so instead of rendering nothing', async function (assert) {