Prevent create-file modal from closing during creation#4298
Prevent create-file modal from closing during creation#4298jurgenwerk wants to merge 1 commit intomainfrom
Conversation
The modal's onCancel now returns early if any creation task is running, preventing the crash caused by accessing newFileDeferred on an already- cleared currentRequest. Fixes CS-10557 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Preview deployments |
There was a problem hiding this comment.
Pull request overview
Prevents the “Create file/card” modal in operator code mode from being dismissed while a create/duplicate operation is in-flight, avoiding a broken modal state on subsequent opens.
Changes:
- Block modal cancellation/dismissal when a create task (or duplicate task) is running.
- Add an acceptance test that simulates an in-flight create request and verifies the modal remains open until completion.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/host/app/components/operator-mode/create-file-modal.gts | Guards onCancel so the modal cannot be closed while create/duplicate tasks are running. |
| packages/host/tests/acceptance/code-submode/create-file-test.gts | Adds an acceptance test that blocks the save POST request and attempts to dismiss the modal during creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (this.isCreateRunning || this.duplicateCardInstance.isRunning) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
onCancel now no-ops while a create/duplicate task is running, but the UI controls that trigger it (Cancel button, close “X”, overlay/escape) will still look interactive and just do nothing. Consider also reflecting the blocked-close state in the UI (e.g., disable the Cancel/close controls and/or show a short “Creating…” hint) so keyboard/screen-reader users get correct affordances.
| if (this.isCreateRunning || this.duplicateCardInstance.isRunning) { | |
| return; | |
| } |
| @action private onCancel() { | ||
| if (this.isCreateRunning || this.duplicateCardInstance.isRunning) { |
There was a problem hiding this comment.
The “cannot close while busy” condition is split between isCreateRunning and an extra duplicateCardInstance.isRunning check here. To keep the busy/disabled logic consistent across the component, consider folding duplicateCardInstance.isRunning into isCreateRunning (or introducing a single isBusy getter) and use that everywhere.
| @action private onCancel() { | |
| if (this.isCreateRunning || this.duplicateCardInstance.isRunning) { | |
| private get isBusy() { | |
| return this.isCreateRunning || this.duplicateCardInstance.isRunning; | |
| } | |
| @action private onCancel() { | |
| if (this.isBusy) { |
| let createClickPromise = click('[data-test-create-definition]'); | ||
|
|
||
| // Wait for the POST to be intercepted (task is now mid-creation) | ||
| await waitUntil(() => postIntercepted); |
There was a problem hiding this comment.
waitUntil(() => postIntercepted) uses the helper’s default timeout, which can make this acceptance test flaky on slower CI runs. Please pass an explicit timeout and timeoutMessage so failures are deterministic and easier to diagnose.
| await waitUntil(() => postIntercepted); | |
| await waitUntil(() => postIntercepted, { | |
| timeout: 10000, | |
| timeoutMessage: | |
| 'Timed out waiting for save POST to be intercepted in create-file-test (modal cannot be dismissed while creation is in progress).', | |
| }); |
| // settled, which won't resolve while the creation task is blocked) | ||
| let cancelBtn = document.querySelector( | ||
| '[data-test-cancel-create-file]', | ||
| ) as HTMLElement; |
There was a problem hiding this comment.
document.querySelector('[data-test-cancel-create-file]') as HTMLElement will throw a less-informative error if the selector ever changes (null deref on .click()). Prefer asserting it exists (or using a non-null check with a clear failure message) before clicking to make this test easier to debug.
| ) as HTMLElement; | |
| ); | |
| if (!cancelBtn) { | |
| throw new Error( | |
| 'Could not find [data-test-cancel-create-file] button while creation is in progress', | |
| ); | |
| } |
There is a scenario where if you choose to create a card or file using the modal in code mode, and click "Create" and then immediately close the modal, this error would show up next time you open the modal:
I think we should not allow the user to close the modal if file creation is still in progress. This fixes the problem.