Fix: memory and context views mobile friendly#818
Merged
gsxdsm merged 12 commits intoAutoMaker-Org:v1.0.0rcfrom Feb 26, 2026
Merged
Fix: memory and context views mobile friendly#818gsxdsm merged 12 commits intoAutoMaker-Org:v1.0.0rcfrom
gsxdsm merged 12 commits intoAutoMaker-Org:v1.0.0rcfrom
Conversation
Code review improvements: - Extract isMarkdownFilename and isImageFilename to shared image-utils.ts - Remove duplicated code from context-view.tsx and memory-view.tsx - Add path traversal guard for context fixture utilities (matching memory) - Add 7 new tests for context fixture path traversal protection - Total 61 tests pass Addresses code review feedback from PR AutoMaker-Org#813 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughAdds mobile-responsive layouts to context and memory views, centralizes filename/type detection utilities, increases Node.js heap for Playwright runs, and introduces extensive Playwright E2E tests and test utilities for context, memory, and profiles plus fixture and navigation improvements. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Summary of ChangesHello @gsxdsm, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience for Memory and Context views by introducing mobile-friendly layouts and navigation. The changes ensure that users can seamlessly interact with files on smaller screens, with intelligent panel management and optimized toolbar actions. Additionally, the underlying code for file type detection has been centralized, and the testing infrastructure has been robustly expanded to cover these new responsive behaviors and edge cases, leading to a more reliable and accessible application. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements by making the Memory and Context views mobile-friendly. The changes include responsive layouts, icon-only buttons for smaller screens, and a mobile-specific back navigation. The refactoring of file type detection into a shared utility is a great move for code consistency. The addition of a comprehensive suite of E2E tests for both mobile and desktop views is excellent and greatly increases confidence in these changes. I've identified one area for future improvement regarding code duplication between the two views.
Comment on lines
+47
to
+56
| // Responsive layout classes | ||
| const FILE_LIST_BASE_CLASSES = 'border-r border-border flex flex-col overflow-hidden'; | ||
| const FILE_LIST_DESKTOP_CLASSES = 'w-64'; | ||
| const FILE_LIST_EXPANDED_CLASSES = 'flex-1'; | ||
| const FILE_LIST_MOBILE_NO_SELECTION_CLASSES = 'w-full border-r-0'; | ||
| const FILE_LIST_MOBILE_SELECTION_CLASSES = 'hidden'; | ||
|
|
||
| const EDITOR_PANEL_BASE_CLASSES = 'flex-1 flex flex-col overflow-hidden'; | ||
| const EDITOR_PANEL_MOBILE_HIDDEN_CLASSES = 'hidden'; | ||
|
|
Contributor
There was a problem hiding this comment.
There is a significant amount of duplicated logic and structure between MemoryView and ContextView. This includes:
- State management (isLoading, isSaving, selectedFile, etc.)
- These responsive layout constants and the
cn()logic that uses them. - Toolbar implementation with responsive buttons.
- Dialogs for creating, renaming, and deleting files.
While this PR's focus is on adding mobile-friendliness, a future refactoring to extract this shared functionality into a reusable component or a set of hooks would greatly improve maintainability. For example, a generic FileEditorView component could be created that takes the specific file handling logic as props.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
apps/ui/tests/memory/mobile-memory-operations.spec.ts (2)
104-105: Replace fixed timeout with dropdown visibility check.The
waitForTimeout(300)after clicking the menu button could be replaced with waiting for the dropdown menu to appear:♻️ Suggested improvement
await fileMenuButton.click({ force: true }); - // Wait for dropdown - await page.waitForTimeout(300); + // Wait for dropdown to appear + await page.locator('[role="menu"], [data-radix-menu-content]').waitFor({ state: 'visible', timeout: 2000 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/tests/memory/mobile-memory-operations.spec.ts` around lines 104 - 105, Replace the fixed sleep (await page.waitForTimeout(300)) with an explicit wait for the dropdown to be visible; locate the dropdown used after clicking the menu button (replace the timeout call in the test in mobile-memory-operations.spec.ts) and use Playwright's wait-for-selector or locator.waitFor with state: 'visible' to wait for that dropdown element instead of a hardcoded timeout.
94-95: Consider replacingwaitForTimeoutwith condition-based waits.Using
page.waitForTimeout(300)can lead to flaky tests. Consider waiting for a specific condition instead, such as verifying the actions panel is hidden:await page.locator('[data-testid="header-actions-backdrop"]').waitFor({ state: 'hidden', timeout: 1000 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/tests/memory/mobile-memory-operations.spec.ts` around lines 94 - 95, Replace the brittle sleep after page.keyboard.press('Escape') by waiting for the actions panel to actually hide: instead of page.waitForTimeout(300) use the page.locator for '[data-testid="header-actions-backdrop"]' and wait for its state to become hidden with a short timeout (e.g. 1000ms) so the test proceeds only when the UI is ready.apps/ui/tests/utils/project/fixtures.ts (1)
199-208: Consider using or removing the unused_optionsparameter.The
_optionsparameter is declared but unused. If profile customization is planned for the future, consider adding a TODO comment. Otherwise, the parameter could be removed until needed.export async function setupMockProjectWithProfiles( page: Page, - _options?: { customProfilesCount?: number } + // TODO: Add profile customization support when needed + _options?: { customProfilesCount?: number } ): Promise<void> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/tests/utils/project/fixtures.ts` around lines 199 - 208, The function setupMockProjectWithProfiles declares an unused parameter _options; either remove the parameter from the signature or mark it as intentionally unused with a TODO comment and keep the underscore, e.g., update the setupMockProjectWithProfiles(page: Page) signature if not needed, or add a brief TODO next to _options noting planned customProfilesCount handling and reference to setupProjectWithFixture and FIXTURE_PATH so future work knows where to plug it in.apps/ui/tests/utils/navigation/views.ts (1)
70-76: Prefer deterministic backdrop-close waiting over fixed sleep.On Line 75, the hardcoded
200msdelay can still race on slower runs. Waiting for the backdrop to become hidden is more stable.♻️ Suggested change
const backdrop = page.locator('[data-testid="sidebar-backdrop"]'); if (await backdrop.isVisible().catch(() => false)) { await backdrop.evaluate((el) => (el as HTMLElement).click()); - await page.waitForTimeout(200); + await backdrop.waitFor({ state: 'hidden', timeout: 2000 }).catch(() => {}); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/tests/utils/navigation/views.ts` around lines 70 - 76, Replace the fixed 200ms sleep with a deterministic wait for the backdrop to be hidden: after clicking the backdrop (variable backdrop), await the locator to reach a hidden/detached state (e.g., await backdrop.waitFor({ state: 'hidden' }) or await page.waitForSelector('[data-testid="sidebar-backdrop"]', { state: 'hidden' })) so the test no longer races on slow runs.apps/ui/tests/memory/file-extension-edge-cases.spec.ts (1)
131-133: Same issue as context tests: avoid arbitrarywaitForTimeout.Use deterministic waits instead of the 1-second timeout which could cause flakiness.
♻️ Suggested improvement
- // Wait a moment for files to load - await page.waitForTimeout(1000); + // Wait for the file list to be visible and stable + const fileList = page.locator('[data-testid="memory-file-list"]'); + await expect(fileList).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/tests/memory/file-extension-edge-cases.spec.ts` around lines 131 - 133, Replace the arbitrary sleep call page.waitForTimeout(1000) with a deterministic wait that asserts the condition that files have loaded; e.g., use page.waitForSelector(...) for the specific file list or file item selector, page.waitForFunction(...) to poll a DOM/state predicate indicating loading is complete, or page.waitForLoadState('networkidle') if appropriate—locate the call to page.waitForTimeout in the test and swap it for one of these deterministic waits targeting the page element or state that confirms files are ready.apps/ui/tests/context/mobile-context-view.spec.ts (1)
198-216: Consider simplifying the icon visibility check.The async IIFE pattern works but could be simplified using Playwright's built-in
orlocator for cleaner assertions.♻️ Alternative approach using `or` locator
- // One of the icons should be present - const hasIcon = await (async () => { - const eyeVisible = await eyeIcon.isVisible().catch(() => false); - const pencilVisible = await pencilIcon.isVisible().catch(() => false); - return eyeVisible || pencilVisible; - })(); - - expect(hasIcon).toBe(true); + // One of the icons should be present + const iconLocator = toggleButton.locator('svg.lucide-eye, svg.lucide-pencil'); + await expect(iconLocator).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/tests/context/mobile-context-view.spec.ts` around lines 198 - 216, Replace the async IIFE icon check with a single combined locator for cleanliness: use toggleButton.locator('svg.lucide-eye, svg.lucide-pencil') (or Playwright's locator OR variant) to get a single "icons" locator and then assert its visibility (e.g., await icons.isVisible() or expect(icons).toBeVisible()), removing the eyeIcon/pencilIcon try/catch logic and the IIFE; refer to the existing eyeIcon, pencilIcon and toggleButton locators to locate and replace the logic.apps/ui/tests/memory/mobile-memory-view.spec.ts (1)
198-212: Same optional simplification as context mobile tests.The icon visibility check could use Playwright's CSS selector combination for cleaner code.
♻️ Alternative approach
- // One of the icons should be present - const hasIcon = await (async () => { - const eyeVisible = await eyeIcon.isVisible().catch(() => false); - const pencilVisible = await pencilIcon.isVisible().catch(() => false); - return eyeVisible || pencilVisible; - })(); - - expect(hasIcon).toBe(true); + // One of the icons should be present + const iconLocator = toggleButton.locator('svg.lucide-eye, svg.lucide-pencil'); + await expect(iconLocator).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/tests/memory/mobile-memory-view.spec.ts` around lines 198 - 212, Replace the manual async IIFE that checks eyeIcon and pencilIcon visibility with a single Playwright locator using a combined CSS selector from the toggleButton (e.g., toggleButton.locator('svg, .eye-icon, .pencil-icon')) and call isVisible() on that locator to produce hasIcon; update references to eyeIcon/pencilIcon accordingly so the test reads: hasIcon = await toggleButton.locator('<combined-selector>').isVisible().catch(() => false).apps/ui/tests/context/file-extension-edge-cases.spec.ts (1)
131-133: Avoid arbitrarywaitForTimeout- use deterministic waits instead.The 1-second timeout on line 133 could cause test flakiness. Consider using a more deterministic approach like waiting for the file list to stabilize or checking that loading has completed.
♻️ Suggested improvement
- // Wait a moment for files to load - await page.waitForTimeout(1000); + // Wait for the file list to be visible and stable + const fileList = page.locator('[data-testid="context-file-list"]'); + await expect(fileList).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/tests/context/file-extension-edge-cases.spec.ts` around lines 131 - 133, Replace the arbitrary await page.waitForTimeout(1000); call with a deterministic wait that ensures the file list has finished loading: remove the page.waitForTimeout invocation and instead wait for a concrete condition such as a specific selector that indicates loading finished (e.g. absence of a spinner or presence of the file list rows) or use page.waitForResponse / page.waitForLoadState('networkidle') as appropriate; update the test in file-extension-edge-cases.spec.ts by targeting the same "page" object and replacing the waitForTimeout call with one of these deterministic waits so the test only proceeds once loading is complete.apps/ui/src/components/views/memory-view.tsx (1)
84-86: Consider documenting the unsaved changes behavior for future reference.The comment on lines 144-146 mentions that unsaved changes warning could be added in the future. This is acceptable for the current mobile UX flow, but consider creating an issue to track this enhancement if data loss is a concern.
Also applies to: 143-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/views/memory-view.tsx` around lines 84 - 86, Add a short actionable note and a tracked issue for the "unsaved changes" behavior referenced around the Mobile detection/UX in apps/ui/src/components/views/memory-view.tsx (the isMobile constant / MemoryView component): create a repository issue describing the desired unsaved-changes warning behavior and data-loss scenarios, then update the inline comment near the existing note (around the unsaved changes comment at lines ~143-148) to reference the new issue number (e.g., "TODO: track unsaved-changes warning — see issue `#123`"), so future contributors can find the enhancement request easily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ui/src/components/views/context-view.tsx`:
- Around line 201-206: The file listing filter currently only keeps entries
matching isMarkdownFilename or isImageFilename (and excludes
'context-metadata.json'), which hides other valid uploaded text files; update
the filter to also accept generic text files (e.g., add an
isTextFilename(entry.name) or a text-mime check) so the client-side list uses
the same validation as the uploader, and reuse the same helper used by the
upload flow (the upload validation/handler) to ensure consistency between the
upload code and the .filter that currently references isMarkdownFilename and
isImageFilename.
- Around line 243-247: The change removed the unsaved-edits safeguard when
switching files; restore a guard in the file-switch handler that checks the
editor dirty flag (e.g., isDirty or hasUnsavedChanges) before calling
loadFileContent(file) and setIsPreviewMode(...). If dirty, show a confirmation
modal or browser confirm and only proceed to call loadFileContent and
setIsPreviewMode when the user confirms (or provide a “Save first” action); keep
the safeguard logic colocated with the existing handler that currently invokes
loadFileContent and setIsPreviewMode.
In `@apps/ui/tests/context/file-extension-edge-cases.spec.ts`:
- Around line 139-168: The test 'should NOT create file without .md extension
via UI' currently swallows failures with a .catch(() => {}) on the expect call
for fileButton; remove that catch so the assertion actually fails when the file
appears, and update the assertion to match the intended behavior: if the UI
should auto-append .md, replace the locator
`[data-testid="context-file-${fileName}"]` with
`[data-testid="context-file-${fileName}.md"]` and assert visibility
(expect(...).toBeVisible) or, if the UI should prevent creation, keep
`[data-testid="context-file-${fileName}"]` and assert not visible
(expect(...).not.toBeVisible) without any .catch; adjust the test body around
clickElement('confirm-create-markdown') and the locator usage accordingly.
In `@apps/ui/tests/context/mobile-context-operations.spec.ts`:
- Around line 97-103: The test is hiding real mobile behavior by using
fileRow.hover() and fileMenuButton.click({ force: true }); update the test to
emulate touch access: remove hover() and the forced click, instead perform an
interaction that a mobile user would do (e.g., await fileRow.click() or
tap-equivalent) to reveal the menu, then wait for the menu trigger to be
visible/attached and click it normally (no force). Use the existing locators
(fileRow -> `[data-testid="context-file-${fileName}"]` and fileMenuButton ->
`[data-testid="context-file-menu-${fileName}"]`) and add an assertion/wait for
visibility before invoking fileMenuButton.click().
In `@apps/ui/tests/memory/file-extension-edge-cases.spec.ts`:
- Around line 139-167: The test's .catch(() => {}) swallows assertion failures
making the check ineffective; remove the catch and assert deterministically:
after creating the memory, check for the presence/absence of the file by looking
for both possible locators
(page.locator(`[data-testid="memory-file-${fileName}"]`) and
page.locator(`[data-testid="memory-file-${fileName}.md"]`)) and assert the
expected one (use expect(...).not.toBeVisible or expect(...).toBeVisible)
instead of swallowing errors; update the assertions that reference fileButton
and the locator logic used around fillInput, clickElement and waitForNetworkIdle
to test the actual UI behavior (auto-append .md) without catching assertion
failures.
In `@apps/ui/tests/utils/project/fixtures.spec.ts`:
- Around line 51-63: The test currently treats Windows-style backslash sequences
("..\\") as acceptable which makes traversal protection platform-dependent;
update the test for "should handle Windows-style path traversal attempt ..\\
(platform-dependent)" to normalize incoming paths (treat backslashes as
separators) before calling memoryFileExistsOnDisk and assert that a
backslash-based traversal input is rejected/throws just like a forward-slash
traversal; locate the test and the memoryFileExistsOnDisk usage and change the
assertion to enforce that inputs containing backslash traversal are normalized
and blocked rather than allowed.
In `@apps/ui/tests/utils/views/context.ts`:
- Around line 100-115: The waitForFunction predicate that checks the save button
(selector '[data-testid="save-context-file"]' and local variable btn) only looks
for the string "saved" in text/aria/title, which times out on icon-only mobile
controls; update the predicate so it returns true if either the current
text/aria/title contains "saved" OR the button indicates completion via state
attributes (e.g., btn.disabled === true OR btn.getAttribute('aria-disabled') ===
'true' OR btn.getAttribute('aria-busy') === 'false') or there is no saving
spinner inside the button (e.g., btn.querySelector('.spinner') === null); modify
the anonymous function passed to page.waitForFunction accordingly so mobile
icon-only controls are treated as saved.
In `@apps/ui/tests/utils/views/profiles.ts`:
- Around line 160-165: The code in saveProfile (and the similar block at
170-177) swallows failures from waitForElementHidden, masking dialog-close
regressions; remove the .catch(() => {}) suppression and either (A) await
waitForElementHidden(...) directly so errors propagate, or (B) first check
dialog presence (e.g., an isElementVisible/exists helper) and only call
waitForElementHidden for dialogs that are present. Update the saveProfile
function (references: clickElement(page, 'save-profile-button'),
waitForElementHidden(page, 'add-profile-dialog'), waitForElementHidden(page,
'edit-profile-dialog')) and the analogous function at 170-177 to implement one
of these approaches so test failures surface when dialogs fail to close.
- Around line 441-445: The validation in dragProfile only checks upper bounds
and misses negative indices; update the function (dragProfile) to explicitly
check that fromIndex and toIndex are not negative (e.g., fromIndex < 0 or
toIndex < 0) and throw the same informative Error used for out-of-range indices
including the values of fromIndex, toIndex, and cards.length so invalid negative
indices fail fast with a clear message.
In
`@test/opus-thinking-level-none-83449-2bgz7j1/test-project-1772086066067/package.json`:
- Around line 1-4: Remove the committed test artifact directory and package.json
generated by tests, add .gitignore entries to exclude these runtime test
directories, and verify/repair the test cleanup call; specifically, delete the
committed test/opus-thinking-level-none-*/ directory (the package.json shown),
add the following patterns to .gitignore: test/**/test-project-[0-9]*/ ,
test/opus-thinking-*/ , test/agent-session-test-*/ ,
test/feature-backlog-test-*/ , test/running-task-display-test-*/ , and confirm
the test harness' cleanupTempDir() used in
apps/ui/tests/features/opus-thinking-level-none.spec.ts is invoked in
test.afterAll() (or fix its invocation/implementation) so temporary dirs are
removed at runtime.
---
Nitpick comments:
In `@apps/ui/src/components/views/memory-view.tsx`:
- Around line 84-86: Add a short actionable note and a tracked issue for the
"unsaved changes" behavior referenced around the Mobile detection/UX in
apps/ui/src/components/views/memory-view.tsx (the isMobile constant / MemoryView
component): create a repository issue describing the desired unsaved-changes
warning behavior and data-loss scenarios, then update the inline comment near
the existing note (around the unsaved changes comment at lines ~143-148) to
reference the new issue number (e.g., "TODO: track unsaved-changes warning — see
issue `#123`"), so future contributors can find the enhancement request easily.
In `@apps/ui/tests/context/file-extension-edge-cases.spec.ts`:
- Around line 131-133: Replace the arbitrary await page.waitForTimeout(1000);
call with a deterministic wait that ensures the file list has finished loading:
remove the page.waitForTimeout invocation and instead wait for a concrete
condition such as a specific selector that indicates loading finished (e.g.
absence of a spinner or presence of the file list rows) or use
page.waitForResponse / page.waitForLoadState('networkidle') as appropriate;
update the test in file-extension-edge-cases.spec.ts by targeting the same
"page" object and replacing the waitForTimeout call with one of these
deterministic waits so the test only proceeds once loading is complete.
In `@apps/ui/tests/context/mobile-context-view.spec.ts`:
- Around line 198-216: Replace the async IIFE icon check with a single combined
locator for cleanliness: use toggleButton.locator('svg.lucide-eye,
svg.lucide-pencil') (or Playwright's locator OR variant) to get a single "icons"
locator and then assert its visibility (e.g., await icons.isVisible() or
expect(icons).toBeVisible()), removing the eyeIcon/pencilIcon try/catch logic
and the IIFE; refer to the existing eyeIcon, pencilIcon and toggleButton
locators to locate and replace the logic.
In `@apps/ui/tests/memory/file-extension-edge-cases.spec.ts`:
- Around line 131-133: Replace the arbitrary sleep call
page.waitForTimeout(1000) with a deterministic wait that asserts the condition
that files have loaded; e.g., use page.waitForSelector(...) for the specific
file list or file item selector, page.waitForFunction(...) to poll a DOM/state
predicate indicating loading is complete, or
page.waitForLoadState('networkidle') if appropriate—locate the call to
page.waitForTimeout in the test and swap it for one of these deterministic waits
targeting the page element or state that confirms files are ready.
In `@apps/ui/tests/memory/mobile-memory-operations.spec.ts`:
- Around line 104-105: Replace the fixed sleep (await page.waitForTimeout(300))
with an explicit wait for the dropdown to be visible; locate the dropdown used
after clicking the menu button (replace the timeout call in the test in
mobile-memory-operations.spec.ts) and use Playwright's wait-for-selector or
locator.waitFor with state: 'visible' to wait for that dropdown element instead
of a hardcoded timeout.
- Around line 94-95: Replace the brittle sleep after
page.keyboard.press('Escape') by waiting for the actions panel to actually hide:
instead of page.waitForTimeout(300) use the page.locator for
'[data-testid="header-actions-backdrop"]' and wait for its state to become
hidden with a short timeout (e.g. 1000ms) so the test proceeds only when the UI
is ready.
In `@apps/ui/tests/memory/mobile-memory-view.spec.ts`:
- Around line 198-212: Replace the manual async IIFE that checks eyeIcon and
pencilIcon visibility with a single Playwright locator using a combined CSS
selector from the toggleButton (e.g., toggleButton.locator('svg, .eye-icon,
.pencil-icon')) and call isVisible() on that locator to produce hasIcon; update
references to eyeIcon/pencilIcon accordingly so the test reads: hasIcon = await
toggleButton.locator('<combined-selector>').isVisible().catch(() => false).
In `@apps/ui/tests/utils/navigation/views.ts`:
- Around line 70-76: Replace the fixed 200ms sleep with a deterministic wait for
the backdrop to be hidden: after clicking the backdrop (variable backdrop),
await the locator to reach a hidden/detached state (e.g., await
backdrop.waitFor({ state: 'hidden' }) or await
page.waitForSelector('[data-testid="sidebar-backdrop"]', { state: 'hidden' }))
so the test no longer races on slow runs.
In `@apps/ui/tests/utils/project/fixtures.ts`:
- Around line 199-208: The function setupMockProjectWithProfiles declares an
unused parameter _options; either remove the parameter from the signature or
mark it as intentionally unused with a TODO comment and keep the underscore,
e.g., update the setupMockProjectWithProfiles(page: Page) signature if not
needed, or add a brief TODO next to _options noting planned customProfilesCount
handling and reference to setupProjectWithFixture and FIXTURE_PATH so future
work knows where to plug it in.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
apps/ui/playwright.config.tsapps/ui/src/components/ui/header-actions-panel.tsxapps/ui/src/components/views/context-view.tsxapps/ui/src/components/views/memory-view.tsxapps/ui/src/lib/image-utils.tsapps/ui/tests/context/desktop-context-view.spec.tsapps/ui/tests/context/file-extension-edge-cases.spec.tsapps/ui/tests/context/mobile-context-operations.spec.tsapps/ui/tests/context/mobile-context-view.spec.tsapps/ui/tests/memory/desktop-memory-view.spec.tsapps/ui/tests/memory/file-extension-edge-cases.spec.tsapps/ui/tests/memory/mobile-memory-operations.spec.tsapps/ui/tests/memory/mobile-memory-view.spec.tsapps/ui/tests/profiles/profiles-crud.spec.tsapps/ui/tests/projects/board-background-persistence.spec.tsapps/ui/tests/utils/core/interactions.tsapps/ui/tests/utils/core/waiting.tsapps/ui/tests/utils/git/worktree.tsapps/ui/tests/utils/index.tsapps/ui/tests/utils/navigation/views.tsapps/ui/tests/utils/project/fixtures.spec.tsapps/ui/tests/utils/project/fixtures.tsapps/ui/tests/utils/project/setup.tsapps/ui/tests/utils/views/context.tsapps/ui/tests/utils/views/memory.tsapps/ui/tests/utils/views/profiles.tstest/opus-thinking-level-none-83449-2bgz7j1/test-project-1772086066067/package.jsontest/running-task-display-test-805-6c4ockc/test-project-1772088506096/README.mdtest/running-task-display-test-805-6c4ockc/test-project-1772088506096/test-feature.jstest/running-task-display-test-805-6c4ockc/test-project-1772088506096/test-feature.test.js
Comment on lines
+243
to
247
| // Note: Unsaved changes warning could be added here in the future | ||
| // For now, silently proceed to avoid disrupting mobile UX flow | ||
| loadFileContent(file); | ||
| setIsPreviewMode(isMarkdownFile(file.name)); | ||
| setIsPreviewMode(isMarkdownFilename(file.name)); | ||
| }; |
Contributor
There was a problem hiding this comment.
Unsaved edits can now be lost without confirmation.
Line 243–247 removes the unsaved-change safeguard; switching files can discard in-progress content with no prompt.
🧯 Suggested guard
const handleSelectFile = (file: ContextFile) => {
- // Note: Unsaved changes warning could be added here in the future
- // For now, silently proceed to avoid disrupting mobile UX flow
+ if (hasChanges && selectedFile?.path !== file.path) {
+ const shouldDiscard = window.confirm('Discard unsaved changes?');
+ if (!shouldDiscard) return;
+ }
loadFileContent(file);
setIsPreviewMode(isMarkdownFilename(file.name));
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/src/components/views/context-view.tsx` around lines 243 - 247, The
change removed the unsaved-edits safeguard when switching files; restore a guard
in the file-switch handler that checks the editor dirty flag (e.g., isDirty or
hasUnsavedChanges) before calling loadFileContent(file) and
setIsPreviewMode(...). If dirty, show a confirmation modal or browser confirm
and only proceed to call loadFileContent and setIsPreviewMode when the user
confirms (or provide a “Save first” action); keep the safeguard logic colocated
with the existing handler that currently invokes loadFileContent and
setIsPreviewMode.
Comment on lines
+139
to
+168
|
|
||
| test('should NOT create file without .md extension via UI', async ({ page }) => { | ||
| const fileName = 'NOTES'; | ||
| const content = '# Notes without extension'; | ||
|
|
||
| await setupProjectWithFixture(page, getFixturePath()); | ||
| await authenticateForTests(page); | ||
| await navigateToContext(page); | ||
|
|
||
| // Create file via UI without extension | ||
| await clickElement(page, 'create-markdown-button'); | ||
| await page.waitForSelector('[data-testid="create-markdown-dialog"]', { timeout: 5000 }); | ||
|
|
||
| await fillInput(page, 'new-markdown-name', fileName); | ||
| await fillInput(page, 'new-markdown-content', content); | ||
|
|
||
| await clickElement(page, 'confirm-create-markdown'); | ||
| await waitForElementHidden(page, 'create-markdown-dialog', { timeout: 5000 }); | ||
|
|
||
| await waitForNetworkIdle(page); | ||
|
|
||
| // File should NOT appear in list because UI enforces .md extension | ||
| // (The UI may add .md automatically or show validation error) | ||
| const fileButton = page.locator(`[data-testid="context-file-${fileName}"]`); | ||
| await expect(fileButton) | ||
| .not.toBeVisible({ timeout: 3000 }) | ||
| .catch(() => { | ||
| // It's OK if it doesn't appear - that's expected behavior | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Test assertion is ineffective due to catch block swallowing failures.
The .catch(() => {}) on lines 165-167 makes this test always pass regardless of whether the file appears or not. If the intent is to verify the UI auto-appends .md, the test should assert on context-file-${fileName}.md instead.
🐛 Suggested fix
- // File should NOT appear in list because UI enforces .md extension
- // (The UI may add .md automatically or show validation error)
- const fileButton = page.locator(`[data-testid="context-file-${fileName}"]`);
- await expect(fileButton)
- .not.toBeVisible({ timeout: 3000 })
- .catch(() => {
- // It's OK if it doesn't appear - that's expected behavior
- });
+ // The UI auto-appends .md extension, so file should appear as NOTES.md
+ const fileWithExtension = page.locator(`[data-testid="context-file-${fileName}.md"]`);
+ await expect(fileWithExtension).toBeVisible({ timeout: 5000 });
+
+ // The original name without extension should NOT appear
+ const fileWithoutExtension = page.locator(`[data-testid="context-file-${fileName}"]`);
+ await expect(fileWithoutExtension).not.toBeVisible();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test('should NOT create file without .md extension via UI', async ({ page }) => { | |
| const fileName = 'NOTES'; | |
| const content = '# Notes without extension'; | |
| await setupProjectWithFixture(page, getFixturePath()); | |
| await authenticateForTests(page); | |
| await navigateToContext(page); | |
| // Create file via UI without extension | |
| await clickElement(page, 'create-markdown-button'); | |
| await page.waitForSelector('[data-testid="create-markdown-dialog"]', { timeout: 5000 }); | |
| await fillInput(page, 'new-markdown-name', fileName); | |
| await fillInput(page, 'new-markdown-content', content); | |
| await clickElement(page, 'confirm-create-markdown'); | |
| await waitForElementHidden(page, 'create-markdown-dialog', { timeout: 5000 }); | |
| await waitForNetworkIdle(page); | |
| // File should NOT appear in list because UI enforces .md extension | |
| // (The UI may add .md automatically or show validation error) | |
| const fileButton = page.locator(`[data-testid="context-file-${fileName}"]`); | |
| await expect(fileButton) | |
| .not.toBeVisible({ timeout: 3000 }) | |
| .catch(() => { | |
| // It's OK if it doesn't appear - that's expected behavior | |
| }); | |
| }); | |
| test('should NOT create file without .md extension via UI', async ({ page }) => { | |
| const fileName = 'NOTES'; | |
| const content = '# Notes without extension'; | |
| await setupProjectWithFixture(page, getFixturePath()); | |
| await authenticateForTests(page); | |
| await navigateToContext(page); | |
| // Create file via UI without extension | |
| await clickElement(page, 'create-markdown-button'); | |
| await page.waitForSelector('[data-testid="create-markdown-dialog"]', { timeout: 5000 }); | |
| await fillInput(page, 'new-markdown-name', fileName); | |
| await fillInput(page, 'new-markdown-content', content); | |
| await clickElement(page, 'confirm-create-markdown'); | |
| await waitForElementHidden(page, 'create-markdown-dialog', { timeout: 5000 }); | |
| await waitForNetworkIdle(page); | |
| // The UI auto-appends .md extension, so file should appear as NOTES.md | |
| const fileWithExtension = page.locator(`[data-testid="context-file-${fileName}.md"]`); | |
| await expect(fileWithExtension).toBeVisible({ timeout: 5000 }); | |
| // The original name without extension should NOT appear | |
| const fileWithoutExtension = page.locator(`[data-testid="context-file-${fileName}"]`); | |
| await expect(fileWithoutExtension).not.toBeVisible(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/tests/context/file-extension-edge-cases.spec.ts` around lines 139 -
168, The test 'should NOT create file without .md extension via UI' currently
swallows failures with a .catch(() => {}) on the expect call for fileButton;
remove that catch so the assertion actually fails when the file appears, and
update the assertion to match the intended behavior: if the UI should
auto-append .md, replace the locator `[data-testid="context-file-${fileName}"]`
with `[data-testid="context-file-${fileName}.md"]` and assert visibility
(expect(...).toBeVisible) or, if the UI should prevent creation, keep
`[data-testid="context-file-${fileName}"]` and assert not visible
(expect(...).not.toBeVisible) without any .catch; adjust the test body around
clickElement('confirm-create-markdown') and the locator usage accordingly.
Comment on lines
+97
to
+103
| // Click on the file menu dropdown - hover first to make it visible | ||
| const fileRow = page.locator(`[data-testid="context-file-${fileName}"]`); | ||
| await fileRow.hover(); | ||
|
|
||
| const fileMenuButton = page.locator(`[data-testid="context-file-menu-${fileName}"]`); | ||
| await fileMenuButton.click({ force: true }); | ||
|
|
Contributor
There was a problem hiding this comment.
Force-clicking a hidden menu trigger masks real mobile behavior.
Line 97–103 uses hover() and click({ force: true }), which can pass even if users cannot actually access the menu on touch devices.
🧪 Suggested test expectation
- const fileRow = page.locator(`[data-testid="context-file-${fileName}"]`);
- await fileRow.hover();
-
const fileMenuButton = page.locator(`[data-testid="context-file-menu-${fileName}"]`);
- await fileMenuButton.click({ force: true });
+ await expect(fileMenuButton).toBeVisible({ timeout: 5000 });
+ await fileMenuButton.click();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Click on the file menu dropdown - hover first to make it visible | |
| const fileRow = page.locator(`[data-testid="context-file-${fileName}"]`); | |
| await fileRow.hover(); | |
| const fileMenuButton = page.locator(`[data-testid="context-file-menu-${fileName}"]`); | |
| await fileMenuButton.click({ force: true }); | |
| // Click on the file menu dropdown - hover first to make it visible | |
| const fileMenuButton = page.locator(`[data-testid="context-file-menu-${fileName}"]`); | |
| await expect(fileMenuButton).toBeVisible({ timeout: 5000 }); | |
| await fileMenuButton.click(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/tests/context/mobile-context-operations.spec.ts` around lines 97 -
103, The test is hiding real mobile behavior by using fileRow.hover() and
fileMenuButton.click({ force: true }); update the test to emulate touch access:
remove hover() and the forced click, instead perform an interaction that a
mobile user would do (e.g., await fileRow.click() or tap-equivalent) to reveal
the menu, then wait for the menu trigger to be visible/attached and click it
normally (no force). Use the existing locators (fileRow ->
`[data-testid="context-file-${fileName}"]` and fileMenuButton ->
`[data-testid="context-file-menu-${fileName}"]`) and add an assertion/wait for
visibility before invoking fileMenuButton.click().
Comment on lines
+139
to
+167
| test('should NOT create file without .md extension via UI', async ({ page }) => { | ||
| const fileName = 'NOTES'; | ||
| const content = '# Notes without extension'; | ||
|
|
||
| await setupProjectWithFixture(page, getFixturePath()); | ||
| await authenticateForTests(page); | ||
| await navigateToMemory(page); | ||
|
|
||
| // Create file via UI without extension | ||
| await clickElement(page, 'create-memory-button'); | ||
| await page.waitForSelector('[data-testid="create-memory-dialog"]', { timeout: 5000 }); | ||
|
|
||
| await fillInput(page, 'new-memory-name', fileName); | ||
| await fillInput(page, 'new-memory-content', content); | ||
|
|
||
| await clickElement(page, 'confirm-create-memory'); | ||
| await waitForElementHidden(page, 'create-memory-dialog', { timeout: 5000 }); | ||
|
|
||
| await waitForNetworkIdle(page); | ||
|
|
||
| // File should NOT appear in list because UI enforces .md extension | ||
| // (The UI may add .md automatically or show validation error) | ||
| const fileButton = page.locator(`[data-testid="memory-file-${fileName}"]`); | ||
| await expect(fileButton) | ||
| .not.toBeVisible({ timeout: 3000 }) | ||
| .catch(() => { | ||
| // It's OK if it doesn't appear - that's expected behavior | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Same issue as context tests: catch block makes assertion ineffective.
The .catch(() => {}) swallows failures. If the UI auto-appends .md, assert on the correct filename.
🐛 Suggested fix
- // File should NOT appear in list because UI enforces .md extension
- // (The UI may add .md automatically or show validation error)
- const fileButton = page.locator(`[data-testid="memory-file-${fileName}"]`);
- await expect(fileButton)
- .not.toBeVisible({ timeout: 3000 })
- .catch(() => {
- // It's OK if it doesn't appear - that's expected behavior
- });
+ // The UI auto-appends .md extension, so file should appear as NOTES.md
+ const fileWithExtension = page.locator(`[data-testid="memory-file-${fileName}.md"]`);
+ await expect(fileWithExtension).toBeVisible({ timeout: 5000 });
+
+ // The original name without extension should NOT appear
+ const fileWithoutExtension = page.locator(`[data-testid="memory-file-${fileName}"]`);
+ await expect(fileWithoutExtension).not.toBeVisible();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/tests/memory/file-extension-edge-cases.spec.ts` around lines 139 -
167, The test's .catch(() => {}) swallows assertion failures making the check
ineffective; remove the catch and assert deterministically: after creating the
memory, check for the presence/absence of the file by looking for both possible
locators (page.locator(`[data-testid="memory-file-${fileName}"]`) and
page.locator(`[data-testid="memory-file-${fileName}.md"]`)) and assert the
expected one (use expect(...).not.toBeVisible or expect(...).toBeVisible)
instead of swallowing errors; update the assertions that reference fileButton
and the locator logic used around fillInput, clickElement and waitForNetworkIdle
to test the actual UI behavior (auto-append .md) without catching assertion
failures.
Comment on lines
+51
to
+63
| test('should handle Windows-style path traversal attempt ..\\ (platform-dependent)', () => { | ||
| const maliciousFilename = '..\\..\\..\\windows\\system32\\config'; | ||
|
|
||
| // On Unix/macOS, backslash is treated as a literal character in filenames, | ||
| // not as a path separator, so path.resolve doesn't traverse directories. | ||
| // This test documents that behavior - the guard works for Unix paths, | ||
| // but Windows-style backslashes are handled differently per platform. | ||
| // On macOS/Linux: backslash is a valid filename character | ||
| // On Windows: would need additional normalization to prevent traversal | ||
| expect(() => { | ||
| memoryFileExistsOnDisk(maliciousFilename); | ||
| }).not.toThrow(); | ||
|
|
Contributor
There was a problem hiding this comment.
Do not encode backslash traversal as acceptable.
At Line 60–63, asserting ..\\ is valid effectively blesses a Windows traversal vector and makes guard behavior platform-dependent.
🛡️ Suggested test contract change
- expect(() => {
- memoryFileExistsOnDisk(maliciousFilename);
- }).not.toThrow();
+ expect(() => {
+ memoryFileExistsOnDisk(maliciousFilename);
+ }).toThrow('Invalid memory filename');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/tests/utils/project/fixtures.spec.ts` around lines 51 - 63, The test
currently treats Windows-style backslash sequences ("..\\") as acceptable which
makes traversal protection platform-dependent; update the test for "should
handle Windows-style path traversal attempt ..\\ (platform-dependent)" to
normalize incoming paths (treat backslashes as separators) before calling
memoryFileExistsOnDisk and assert that a backslash-based traversal input is
rejected/throws just like a forward-slash traversal; locate the test and the
memoryFileExistsOnDisk usage and change the assertion to enforce that inputs
containing backslash traversal are normalized and blocked rather than allowed.
Comment on lines
+100
to
+115
| // Wait for save to complete across desktop/mobile variants | ||
| // On desktop: button text shows "Saved" | ||
| // On mobile: icon-only button uses aria-label or title | ||
| await page.waitForFunction( | ||
| () => | ||
| document.querySelector('[data-testid="save-context-file"]')?.textContent?.includes('Saved'), | ||
| () => { | ||
| const btn = document.querySelector('[data-testid="save-context-file"]'); | ||
| if (!btn) return false; | ||
| const stateText = [ | ||
| btn.textContent ?? '', | ||
| btn.getAttribute('aria-label') ?? '', | ||
| btn.getAttribute('title') ?? '', | ||
| ] | ||
| .join(' ') | ||
| .toLowerCase(); | ||
| return stateText.includes('saved'); | ||
| }, |
Contributor
There was a problem hiding this comment.
Save completion check can timeout on mobile.
Line 114 waits for "saved" text, but mobile uses icon-only save controls (aria-label/title stay "Save"). This can fail even when save succeeded.
✅ Suggested stabilization
export async function saveContextFile(page: Page): Promise<void> {
await clickElement(page, 'save-context-file');
- // Wait for save to complete across desktop/mobile variants
- // On desktop: button text shows "Saved"
- // On mobile: icon-only button uses aria-label or title
- await page.waitForFunction(
- () => {
- const btn = document.querySelector('[data-testid="save-context-file"]');
- if (!btn) return false;
- const stateText = [
- btn.textContent ?? '',
- btn.getAttribute('aria-label') ?? '',
- btn.getAttribute('title') ?? '',
- ]
- .join(' ')
- .toLowerCase();
- return stateText.includes('saved');
- },
- { timeout: 5000 }
- );
+ await expect(page.locator('[data-testid="save-context-file"]')).toBeDisabled({
+ timeout: 5000,
+ });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/tests/utils/views/context.ts` around lines 100 - 115, The
waitForFunction predicate that checks the save button (selector
'[data-testid="save-context-file"]' and local variable btn) only looks for the
string "saved" in text/aria/title, which times out on icon-only mobile controls;
update the predicate so it returns true if either the current text/aria/title
contains "saved" OR the button indicates completion via state attributes (e.g.,
btn.disabled === true OR btn.getAttribute('aria-disabled') === 'true' OR
btn.getAttribute('aria-busy') === 'false') or there is no saving spinner inside
the button (e.g., btn.querySelector('.spinner') === null); modify the anonymous
function passed to page.waitForFunction accordingly so mobile icon-only controls
are treated as saved.
Comment on lines
+160
to
+165
| export async function saveProfile(page: Page): Promise<void> { | ||
| await clickElement(page, 'save-profile-button'); | ||
| // Wait for dialog to close | ||
| await waitForElementHidden(page, 'add-profile-dialog').catch(() => {}); | ||
| await waitForElementHidden(page, 'edit-profile-dialog').catch(() => {}); | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t swallow dialog-close failures after save/cancel actions.
Suppressing waitForElementHidden errors can hide broken dialog flows and make tests pass through invalid UI state.
Proposed fix
export async function saveProfile(page: Page): Promise<void> {
+ const isAddOpen = await isAddProfileDialogOpen(page);
+ const isEditOpen = await isEditProfileDialogOpen(page);
+ if (!isAddOpen && !isEditOpen) {
+ throw new Error('No profile dialog is open before save');
+ }
+
await clickElement(page, 'save-profile-button');
- // Wait for dialog to close
- await waitForElementHidden(page, 'add-profile-dialog').catch(() => {});
- await waitForElementHidden(page, 'edit-profile-dialog').catch(() => {});
+ await waitForElementHidden(page, isAddOpen ? 'add-profile-dialog' : 'edit-profile-dialog');
}
export async function cancelProfileDialog(page: Page): Promise<void> {
+ const isAddOpen = await isAddProfileDialogOpen(page);
+ const isEditOpen = await isEditProfileDialogOpen(page);
+ if (!isAddOpen && !isEditOpen) {
+ throw new Error('No profile dialog is open before cancel');
+ }
+
// Look for cancel button in dialog footer
const cancelButton = page.locator('button:has-text("Cancel")');
await cancelButton.click();
- // Wait for dialog to close
- await waitForElementHidden(page, 'add-profile-dialog').catch(() => {});
- await waitForElementHidden(page, 'edit-profile-dialog').catch(() => {});
+ await waitForElementHidden(page, isAddOpen ? 'add-profile-dialog' : 'edit-profile-dialog');
}Also applies to: 170-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/tests/utils/views/profiles.ts` around lines 160 - 165, The code in
saveProfile (and the similar block at 170-177) swallows failures from
waitForElementHidden, masking dialog-close regressions; remove the .catch(() =>
{}) suppression and either (A) await waitForElementHidden(...) directly so
errors propagate, or (B) first check dialog presence (e.g., an
isElementVisible/exists helper) and only call waitForElementHidden for dialogs
that are present. Update the saveProfile function (references:
clickElement(page, 'save-profile-button'), waitForElementHidden(page,
'add-profile-dialog'), waitForElementHidden(page, 'edit-profile-dialog')) and
the analogous function at 170-177 to implement one of these approaches so test
failures surface when dialogs fail to close.
Comment on lines
+441
to
+445
| if (fromIndex >= cards.length || toIndex >= cards.length) { | ||
| throw new Error( | ||
| `Invalid drag indices: fromIndex=${fromIndex}, toIndex=${toIndex}, total=${cards.length}` | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Validate negative drag indices explicitly.
dragProfile only checks upper bounds. Negative indices currently bypass validation and fail later with less-informative runtime errors.
Proposed fix
- if (fromIndex >= cards.length || toIndex >= cards.length) {
+ if (fromIndex < 0 || toIndex < 0 || fromIndex >= cards.length || toIndex >= cards.length) {
throw new Error(
`Invalid drag indices: fromIndex=${fromIndex}, toIndex=${toIndex}, total=${cards.length}`
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ui/tests/utils/views/profiles.ts` around lines 441 - 445, The validation
in dragProfile only checks upper bounds and misses negative indices; update the
function (dragProfile) to explicitly check that fromIndex and toIndex are not
negative (e.g., fromIndex < 0 or toIndex < 0) and throw the same informative
Error used for out-of-range indices including the values of fromIndex, toIndex,
and cards.length so invalid negative indices fail fast with a clear message.
Comment on lines
+1
to
+4
| { | ||
| "name": "test-project-1772086066067", | ||
| "version": "1.0.0" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify test artifact claims
echo "=== 1. Searching for similar test artifact directories ==="
fd -t d -d 4 'opus-thinking' test/ 2>/dev/null | head -20
echo -e "\n=== 2. Searching for timestamped test-project directories ==="
fd -t d 'test-project-[0-9]' test/ 2>/dev/null | head -20
echo -e "\n=== 3. Checking .gitignore for test artifact patterns ==="
if [ -f .gitignore ]; then
cat .gitignore | grep -E 'test|opus|tmp|temp' || echo "No matching patterns found"
else
echo ".gitignore file not found"
fi
echo -e "\n=== 4. Searching for test fixture/artifact generation code ==="
rg -l 'opus-thinking|test-project-[0-9]|random.*test.*dir|generate.*test.*artifact' --type ts --type js --type json -g '!node_modules' 2>/dev/null | head -20
echo -e "\n=== 5. Checking if full path exists and is in git ==="
git ls-files test/opus-thinking-level-none-83449-2bgz7j1/ 2>/dev/null | head -20
echo -e "\n=== 6. Verifying the timestamp ==="
# Convert 1772086066067 milliseconds to date
python3 <<'PYEOF'
import datetime
timestamp_ms = 1772086066067
timestamp_s = timestamp_ms / 1000
dt = datetime.datetime.fromtimestamp(timestamp_s, tz=datetime.timezone.utc)
print(f"Timestamp {timestamp_ms}ms converts to: {dt.isoformat()}")
PYEOFRepository: AutoMaker-Org/automaker
Length of output: 1496
🏁 Script executed:
#!/bin/bash
# Check test directory structure to understand context
echo "=== Listing test directory structure (top level) ==="
ls -la test/ 2>/dev/null | head -20
echo -e "\n=== Looking for test generation/setup code ==="
find test -name "*.ts" -o -name "*.js" 2>/dev/null | grep -E '(setup|fixture|generate|create)' | head -20
echo -e "\n=== Searching for references to this specific package in test files ==="
rg 'test-project-1772086066067|opus-thinking-level-none-83449-2bgz7j1' . --type ts --type js 2>/dev/nullRepository: AutoMaker-Org/automaker
Length of output: 836
🏁 Script executed:
#!/bin/bash
# Check the test spec file that references opus-thinking-level-none
echo "=== Examining opus-thinking-level-none.spec.ts ==="
cat -n apps/ui/tests/features/opus-thinking-level-none.spec.ts
echo -e "\n=== Checking test setup code ==="
cat -n apps/ui/tests/utils/project/setup.ts 2>/dev/null | head -50
echo -e "\n=== Searching for any reference to test artifacts or test-project generation ==="
rg 'test-project|createTestProject|generateTestArtifact|OPUS_THINKING' --type ts --type js -A 3 -B 3 2>/dev/null | head -100Repository: AutoMaker-Org/automaker
Length of output: 14573
🏁 Script executed:
#!/bin/bash
# Check git history to see if these artifacts were intentionally added
echo "=== Checking git status/history of the test artifact directory ==="
git log --oneline -- test/opus-thinking-level-none-83449-2bgz7j1/ 2>/dev/null | head -10
echo -e "\n=== Listing all files in the opus-thinking artifact directory ==="
find test/opus-thinking-level-none-83449-2bgz7j1/ -type f -exec echo {} \;
echo -e "\n=== Checking if test fixture files look like templates or actual content ==="
find test -name 'package.json' -path '*/test-project-*' -exec sh -c 'echo "=== {} ==="; cat "$1"' _ {} \; | head -80Repository: AutoMaker-Org/automaker
Length of output: 1056
Remove generated test artifacts and add .gitignore rules.
This directory and its package.json are generated test artifacts created during test execution with dynamic timestamps (via Date.now() in apps/ui/tests/features/opus-thinking-level-none.spec.ts). The test suite creates these at runtime and calls cleanupTempDir() in test.afterAll(), but they're being committed to git when they should be excluded.
Multiple similar test artifact directories exist in the repository (e.g., test/agent-session-test-*/, test/feature-backlog-test-*/, test/running-task-display-test-*/), suggesting a pattern of test cleanup failures or missing .gitignore rules.
Required actions:
- Remove all committed test artifact directories from this PR
- Add patterns to
.gitignoreto prevent future commits:test/**/test-project-[0-9]*/ test/opus-thinking-*/ test/agent-session-test-*/ test/feature-backlog-test-*/ test/running-task-display-test-*/ - Verify
cleanupTempDir()is functioning correctly or troubleshoot test cleanup
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@test/opus-thinking-level-none-83449-2bgz7j1/test-project-1772086066067/package.json`
around lines 1 - 4, Remove the committed test artifact directory and
package.json generated by tests, add .gitignore entries to exclude these runtime
test directories, and verify/repair the test cleanup call; specifically, delete
the committed test/opus-thinking-level-none-*/ directory (the package.json
shown), add the following patterns to .gitignore: test/**/test-project-[0-9]*/ ,
test/opus-thinking-*/ , test/agent-session-test-*/ ,
test/feature-backlog-test-*/ , test/running-task-display-test-*/ , and confirm
the test harness' cleanupTempDir() used in
apps/ui/tests/features/opus-thinking-level-none.spec.ts is invoked in
test.afterAll() (or fix its invocation/implementation) so temporary dirs are
removed at runtime.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/ui/src/components/views/context-view.tsx (1)
237-241:⚠️ Potential issue | 🟠 MajorRestore unsaved-change confirmation before file switches.
Line 238 removed the dirty-state guard, so selecting another file can silently discard edits in progress.
🧯 Suggested fix
const handleSelectFile = (file: ContextFile) => { - // Note: Unsaved changes warning could be added here in the future - // For now, silently proceed to avoid disrupting mobile UX flow + if (hasChanges && selectedFile?.path !== file.path) { + const shouldDiscard = window.confirm('Discard unsaved changes?'); + if (!shouldDiscard) return; + } loadFileContent(file); setIsPreviewMode(isMarkdownFilename(file.name)); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/views/context-view.tsx` around lines 237 - 241, The file-switch handler handleSelectFile currently calls loadFileContent and setIsPreviewMode unconditionally, which discards in-progress edits; restore the dirty-state guard by checking the editor's dirty flag (e.g., isEditorDirty / hasUnsavedChanges) before changing files and show a confirmation modal (e.g., confirmDiscardChanges or showUnsavedChangesModal). Only call loadFileContent(file) and setIsPreviewMode(isMarkdownFilename(file.name)) after the user confirms discard; if the user cancels, abort the switch. Ensure the check and modal wiring live inside handleSelectFile so behavior is identical for all callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ui/src/components/views/context-view.tsx`:
- Around line 969-970: The toolbar's delete button is hidden on mobile via the
isMobile check while the per-row actions remain hover-only (the list-row actions
trigger), making rename/delete inaccessible on touch devices; update
context-view.tsx so either (a) render the toolbar delete button for mobile as a
touch-accessible dropdown/item instead of hiding it when isMobile is true or (b)
make the list-row actions trigger respond to tap (add onClick/onTouchStart and
keyboard handlers) in addition to hover so touch users can open the actions
menu; ensure you modify the isMobile conditional around the toolbar delete and
the list-row actions trigger component to support click/tap and proper ARIA
attributes for discoverability.
---
Duplicate comments:
In `@apps/ui/src/components/views/context-view.tsx`:
- Around line 237-241: The file-switch handler handleSelectFile currently calls
loadFileContent and setIsPreviewMode unconditionally, which discards in-progress
edits; restore the dirty-state guard by checking the editor's dirty flag (e.g.,
isEditorDirty / hasUnsavedChanges) before changing files and show a confirmation
modal (e.g., confirmDiscardChanges or showUnsavedChangesModal). Only call
loadFileContent(file) and setIsPreviewMode(isMarkdownFilename(file.name)) after
the user confirms discard; if the user cancels, abort the switch. Ensure the
check and modal wiring live inside handleSelectFile so behavior is identical for
all callers.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/ui/src/components/views/context-view.tsx (2)
243-247:⚠️ Potential issue | 🟠 MajorRestore unsaved-change confirmation before file switch.
Line 245 loads the next file unconditionally, so in-progress edits are dropped when a different file is selected.
🧯 Proposed fix
const handleSelectFile = (file: ContextFile) => { - // Note: Unsaved changes warning could be added here in the future - // For now, silently proceed to avoid disrupting mobile UX flow + if (hasChanges && selectedFile?.path !== file.path) { + const shouldDiscard = window.confirm('Discard unsaved changes?'); + if (!shouldDiscard) return; + } loadFileContent(file); setIsPreviewMode(isMarkdownFilename(file.name)); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/views/context-view.tsx` around lines 243 - 247, The code currently calls loadFileContent(file) and setIsPreviewMode(...) unconditionally, dropping in-progress edits; update the file-switch handler to first detect unsaved changes (use the existing editor state/unsaved flag or compare current editor text to active file content) and, if dirty, show a confirmation modal/dialog; only call loadFileContent(file) and setIsPreviewMode(isMarkdownFilename(file.name)) after the user confirms, and do nothing (or close the selection) if the user cancels. Ensure you hook the confirm action to the same handler so loadFileContent and setIsPreviewMode are executed only on confirmation.
201-206:⚠️ Potential issue | 🟠 MajorAlign file-list filtering with upload acceptance rules.
Line 205 restricts visible files to markdown/images, but Line 394-405 still allows uploading other non-image text files. Those files can be saved and then disappear from the list after reload.
🐛 Proposed fix
- const files: ContextFile[] = result.entries - .filter( - (entry) => - entry.isFile && - entry.name !== 'context-metadata.json' && - (isMarkdownFilename(entry.name) || isImageFilename(entry.name)) - ) + const files: ContextFile[] = result.entries + .filter((entry) => entry.isFile && entry.name !== 'context-metadata.json') .map((entry) => ({ name: entry.name, type: isImageFilename(entry.name) ? 'image' : 'text', path: `${contextPath}/${entry.name}`, description: metadata.files[entry.name]?.description, }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ui/src/components/views/context-view.tsx` around lines 201 - 206, The file-list filter currently shows only files matching isMarkdownFilename or isImageFilename but the uploader/drop handler still accepts other non-image text files, causing saved uploads to vanish; fix by making the two places use the same predicate: either update the filter to also include the uploader's accepted text-file predicate (add/implement an isTextFilename helper and include it alongside isMarkdownFilename and isImageFilename) or tighten the uploader/drop handler to only accept isMarkdownFilename || isImageFilename; update both the file-list filter and the upload acceptance logic to reference the same shared helper so behavior is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/ui/src/components/views/context-view.tsx`:
- Around line 243-247: The code currently calls loadFileContent(file) and
setIsPreviewMode(...) unconditionally, dropping in-progress edits; update the
file-switch handler to first detect unsaved changes (use the existing editor
state/unsaved flag or compare current editor text to active file content) and,
if dirty, show a confirmation modal/dialog; only call loadFileContent(file) and
setIsPreviewMode(isMarkdownFilename(file.name)) after the user confirms, and do
nothing (or close the selection) if the user cancels. Ensure you hook the
confirm action to the same handler so loadFileContent and setIsPreviewMode are
executed only on confirmation.
- Around line 201-206: The file-list filter currently shows only files matching
isMarkdownFilename or isImageFilename but the uploader/drop handler still
accepts other non-image text files, causing saved uploads to vanish; fix by
making the two places use the same predicate: either update the filter to also
include the uploader's accepted text-file predicate (add/implement an
isTextFilename helper and include it alongside isMarkdownFilename and
isImageFilename) or tighten the uploader/drop handler to only accept
isMarkdownFilename || isImageFilename; update both the file-list filter and the
upload acceptance logic to reference the same shared helper so behavior is
consistent.
gsxdsm
added a commit
to gsxdsm/automaker
that referenced
this pull request
Feb 26, 2026
* Changes from fix/memory-and-context-mobile-friendly * fix: Improve file extension detection and add path traversal protection * refactor: Extract file extension utilities and add path traversal guards Code review improvements: - Extract isMarkdownFilename and isImageFilename to shared image-utils.ts - Remove duplicated code from context-view.tsx and memory-view.tsx - Add path traversal guard for context fixture utilities (matching memory) - Add 7 new tests for context fixture path traversal protection - Total 61 tests pass Addresses code review feedback from PR AutoMaker-Org#813 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: Add e2e tests for profiles crud and board background persistence * Update apps/ui/playwright.config.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: Add robust test navigation handling and file filtering * fix: Format NODE_OPTIONS configuration on single line * test: Update profiles and board background persistence tests * test: Replace iPhone 13 Pro with Pixel 5 for mobile test consistency * Update apps/ui/src/components/views/context-view.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore: Remove test project directory * feat: Filter context files by type and improve mobile menu visibility --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
gsxdsm
added a commit
that referenced
this pull request
Feb 28, 2026
* fix(copilot): correct tool.execution_complete event handling The CopilotProvider was using incorrect event type and data structure for tool execution completion events from the @github/copilot-sdk, causing tool call outputs to be empty. Changes: - Update event type from 'tool.execution_end' to 'tool.execution_complete' - Fix data structure to use nested result.content instead of flat result - Fix error structure to use error.message instead of flat error - Add success field to match SDK event structure - Add tests for empty and missing result handling This aligns with the official @github/copilot-sdk v0.1.16 types defined in session-events.d.ts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(copilot): add edge case test for error with code field Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(copilot): improve error handling and code quality Code review improvements: - Extract magic string '[ERROR]' to TOOL_ERROR_PREFIX constant - Add null-safe error handling with direct error variable assignment - Include error codes in error messages for better debugging - Add JSDoc documentation for tool.execution_complete handler - Update tests to verify error codes are displayed - Add missing tool_use_id assertion in error test These changes improve: - Code maintainability (no magic strings) - Debugging experience (error codes now visible) - Type safety (explicit null checks) - Test coverage (verify error code formatting) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Changes from fix/bug-fixes-1-0 * test(copilot): add edge case test for error with code field Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Changes from fix/bug-fixes-1-0 * fix: Handle detached HEAD state in worktree discovery and recovery * fix: Remove unused isDevServerStarting prop and md: breakpoint classes * fix: Add missing dependency and sanitize persisted cache data * feat: Ensure NODE_ENV is set to test in vitest configs * feat: Configure Playwright to run only E2E tests * fix: Improve PR tracking and dev server lifecycle management * feat: Add settings-based defaults for planning mode, model config, and custom providers. Fixes #816 * feat: Add worktree and branch selector to graph view * fix: Add timeout and error handling for worktree HEAD ref resolution * fix: use absolute icon path and place icon outside asar on Linux The hicolor icon theme index only lists sizes up to 512x512, so an icon installed only at 1024x1024 is invisible to GNOME/KDE's theme resolver, causing both the app launcher and taskbar to show a generic icon. Additionally, BrowserWindow.icon cannot be read by the window manager when the file is inside app.asar. - extraResources: copy logo_larger.png to resources/ (outside asar) so it lands at /opt/Automaker/resources/logo_larger.png on install - linux.desktop.Icon: set to the absolute resources path, bypassing the hicolor theme lookup and its size constraints entirely - icon-manager.ts: on Linux production use process.resourcesPath so BrowserWindow receives a real filesystem path the WM can read directly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use linux.desktop.entry for custom desktop Icon field electron-builder v26 rejects arbitrary keys in linux.desktop — the correct schema wraps custom .desktop overrides inside desktop.entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: set desktop name on Linux so taskbar uses the correct app icon Without app.setDesktopName(), the window manager cannot associate the running Electron process with automaker.desktop. GNOME/KDE fall back to _NET_WM_ICON which defaults to Electron's own bundled icon. Calling app.setDesktopName('automaker.desktop') before any window is created sets the _GTK_APPLICATION_ID hint and XDG app_id so the WM picks up the desktop entry's Icon for the taskbar. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix: memory and context views mobile friendly (#818) * Changes from fix/memory-and-context-mobile-friendly * fix: Improve file extension detection and add path traversal protection * refactor: Extract file extension utilities and add path traversal guards Code review improvements: - Extract isMarkdownFilename and isImageFilename to shared image-utils.ts - Remove duplicated code from context-view.tsx and memory-view.tsx - Add path traversal guard for context fixture utilities (matching memory) - Add 7 new tests for context fixture path traversal protection - Total 61 tests pass Addresses code review feedback from PR #813 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: Add e2e tests for profiles crud and board background persistence * Update apps/ui/playwright.config.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: Add robust test navigation handling and file filtering * fix: Format NODE_OPTIONS configuration on single line * test: Update profiles and board background persistence tests * test: Replace iPhone 13 Pro with Pixel 5 for mobile test consistency * Update apps/ui/src/components/views/context-view.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore: Remove test project directory * feat: Filter context files by type and improve mobile menu visibility --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: Improve test reliability and localhost handling * chore: Use explicit TEST_USE_EXTERNAL_BACKEND env var for server cleanup * feat: Add E2E/CI mock mode for provider factory and auth verification * feat: Add remoteBranch parameter to pull and rebase operations * chore: Enhance E2E testing setup with worker isolation and auth state management - Updated .gitignore to include worker-specific test fixtures. - Modified e2e-tests.yml to implement test sharding for improved CI performance. - Refactored global setup to authenticate once and save session state for reuse across tests. - Introduced worker-isolated fixture paths to prevent conflicts during parallel test execution. - Improved test navigation and loading handling for better reliability. - Updated various test files to utilize new auth state management and fixture paths. * fix: Update Playwright configuration and improve test reliability - Increased the number of workers in Playwright configuration for better parallelism in CI environments. - Enhanced the board background persistence test to ensure dropdown stability by waiting for the list to populate before interaction, improving test reliability. * chore: Simplify E2E test configuration and enhance mock implementations - Updated e2e-tests.yml to run tests in a single shard for streamlined CI execution. - Enhanced unit tests for worktree list handling by introducing a mock for execGitCommand, improving test reliability and coverage. - Refactored setup functions to better manage command mocks for git operations in tests. - Improved error handling in mkdirSafe function to account for undefined stats in certain environments. * refactor: Improve test configurations and enhance error handling - Updated Playwright configuration to clear VITE_SERVER_URL, ensuring the frontend uses the Vite proxy and preventing cookie domain mismatches. - Enhanced MergeRebaseDialog logic to normalize selectedBranch for better handling of various ref formats. - Improved global setup with a more robust backend health check, throwing an error if the backend is not healthy after retries. - Refactored project creation tests to handle file existence checks more reliably. - Added error handling for missing E2E source fixtures to guide setup process. - Enhanced memory navigation to handle sandbox dialog visibility more effectively. * refactor: Enhance Git command execution and improve test configurations - Updated Git command execution to merge environment paths correctly, ensuring proper command execution context. - Refactored the Git initialization process to handle errors more gracefully and ensure user configuration is set before creating the initial commit. - Improved test configurations by updating Playwright test identifiers for better clarity and consistency across different project states. - Enhanced cleanup functions in tests to handle directory removal more robustly, preventing errors during test execution. * fix: Resolve React hooks errors from duplicate instances in dependency tree * style: Format alias configuration for improved readability --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: DhanushSantosh <dhanushsantoshs05@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
gsxdsm
added a commit
that referenced
this pull request
Mar 3, 2026
* Changes from fix/memory-and-context-mobile-friendly * fix: Improve file extension detection and add path traversal protection * refactor: Extract file extension utilities and add path traversal guards Code review improvements: - Extract isMarkdownFilename and isImageFilename to shared image-utils.ts - Remove duplicated code from context-view.tsx and memory-view.tsx - Add path traversal guard for context fixture utilities (matching memory) - Add 7 new tests for context fixture path traversal protection - Total 61 tests pass Addresses code review feedback from PR #813 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: Add e2e tests for profiles crud and board background persistence * Update apps/ui/playwright.config.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: Add robust test navigation handling and file filtering * fix: Format NODE_OPTIONS configuration on single line * test: Update profiles and board background persistence tests * test: Replace iPhone 13 Pro with Pixel 5 for mobile test consistency * Update apps/ui/src/components/views/context-view.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore: Remove test project directory * feat: Filter context files by type and improve mobile menu visibility --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
gsxdsm
added a commit
that referenced
this pull request
Mar 3, 2026
* fix(copilot): correct tool.execution_complete event handling The CopilotProvider was using incorrect event type and data structure for tool execution completion events from the @github/copilot-sdk, causing tool call outputs to be empty. Changes: - Update event type from 'tool.execution_end' to 'tool.execution_complete' - Fix data structure to use nested result.content instead of flat result - Fix error structure to use error.message instead of flat error - Add success field to match SDK event structure - Add tests for empty and missing result handling This aligns with the official @github/copilot-sdk v0.1.16 types defined in session-events.d.ts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(copilot): add edge case test for error with code field Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(copilot): improve error handling and code quality Code review improvements: - Extract magic string '[ERROR]' to TOOL_ERROR_PREFIX constant - Add null-safe error handling with direct error variable assignment - Include error codes in error messages for better debugging - Add JSDoc documentation for tool.execution_complete handler - Update tests to verify error codes are displayed - Add missing tool_use_id assertion in error test These changes improve: - Code maintainability (no magic strings) - Debugging experience (error codes now visible) - Type safety (explicit null checks) - Test coverage (verify error code formatting) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Changes from fix/bug-fixes-1-0 * test(copilot): add edge case test for error with code field Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Changes from fix/bug-fixes-1-0 * fix: Handle detached HEAD state in worktree discovery and recovery * fix: Remove unused isDevServerStarting prop and md: breakpoint classes * fix: Add missing dependency and sanitize persisted cache data * feat: Ensure NODE_ENV is set to test in vitest configs * feat: Configure Playwright to run only E2E tests * fix: Improve PR tracking and dev server lifecycle management * feat: Add settings-based defaults for planning mode, model config, and custom providers. Fixes #816 * feat: Add worktree and branch selector to graph view * fix: Add timeout and error handling for worktree HEAD ref resolution * fix: use absolute icon path and place icon outside asar on Linux The hicolor icon theme index only lists sizes up to 512x512, so an icon installed only at 1024x1024 is invisible to GNOME/KDE's theme resolver, causing both the app launcher and taskbar to show a generic icon. Additionally, BrowserWindow.icon cannot be read by the window manager when the file is inside app.asar. - extraResources: copy logo_larger.png to resources/ (outside asar) so it lands at /opt/Automaker/resources/logo_larger.png on install - linux.desktop.Icon: set to the absolute resources path, bypassing the hicolor theme lookup and its size constraints entirely - icon-manager.ts: on Linux production use process.resourcesPath so BrowserWindow receives a real filesystem path the WM can read directly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use linux.desktop.entry for custom desktop Icon field electron-builder v26 rejects arbitrary keys in linux.desktop — the correct schema wraps custom .desktop overrides inside desktop.entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: set desktop name on Linux so taskbar uses the correct app icon Without app.setDesktopName(), the window manager cannot associate the running Electron process with automaker.desktop. GNOME/KDE fall back to _NET_WM_ICON which defaults to Electron's own bundled icon. Calling app.setDesktopName('automaker.desktop') before any window is created sets the _GTK_APPLICATION_ID hint and XDG app_id so the WM picks up the desktop entry's Icon for the taskbar. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix: memory and context views mobile friendly (#818) * Changes from fix/memory-and-context-mobile-friendly * fix: Improve file extension detection and add path traversal protection * refactor: Extract file extension utilities and add path traversal guards Code review improvements: - Extract isMarkdownFilename and isImageFilename to shared image-utils.ts - Remove duplicated code from context-view.tsx and memory-view.tsx - Add path traversal guard for context fixture utilities (matching memory) - Add 7 new tests for context fixture path traversal protection - Total 61 tests pass Addresses code review feedback from PR #813 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: Add e2e tests for profiles crud and board background persistence * Update apps/ui/playwright.config.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: Add robust test navigation handling and file filtering * fix: Format NODE_OPTIONS configuration on single line * test: Update profiles and board background persistence tests * test: Replace iPhone 13 Pro with Pixel 5 for mobile test consistency * Update apps/ui/src/components/views/context-view.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore: Remove test project directory * feat: Filter context files by type and improve mobile menu visibility --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: Improve test reliability and localhost handling * chore: Use explicit TEST_USE_EXTERNAL_BACKEND env var for server cleanup * feat: Add E2E/CI mock mode for provider factory and auth verification * feat: Add remoteBranch parameter to pull and rebase operations * chore: Enhance E2E testing setup with worker isolation and auth state management - Updated .gitignore to include worker-specific test fixtures. - Modified e2e-tests.yml to implement test sharding for improved CI performance. - Refactored global setup to authenticate once and save session state for reuse across tests. - Introduced worker-isolated fixture paths to prevent conflicts during parallel test execution. - Improved test navigation and loading handling for better reliability. - Updated various test files to utilize new auth state management and fixture paths. * fix: Update Playwright configuration and improve test reliability - Increased the number of workers in Playwright configuration for better parallelism in CI environments. - Enhanced the board background persistence test to ensure dropdown stability by waiting for the list to populate before interaction, improving test reliability. * chore: Simplify E2E test configuration and enhance mock implementations - Updated e2e-tests.yml to run tests in a single shard for streamlined CI execution. - Enhanced unit tests for worktree list handling by introducing a mock for execGitCommand, improving test reliability and coverage. - Refactored setup functions to better manage command mocks for git operations in tests. - Improved error handling in mkdirSafe function to account for undefined stats in certain environments. * refactor: Improve test configurations and enhance error handling - Updated Playwright configuration to clear VITE_SERVER_URL, ensuring the frontend uses the Vite proxy and preventing cookie domain mismatches. - Enhanced MergeRebaseDialog logic to normalize selectedBranch for better handling of various ref formats. - Improved global setup with a more robust backend health check, throwing an error if the backend is not healthy after retries. - Refactored project creation tests to handle file existence checks more reliably. - Added error handling for missing E2E source fixtures to guide setup process. - Enhanced memory navigation to handle sandbox dialog visibility more effectively. * refactor: Enhance Git command execution and improve test configurations - Updated Git command execution to merge environment paths correctly, ensuring proper command execution context. - Refactored the Git initialization process to handle errors more gracefully and ensure user configuration is set before creating the initial commit. - Improved test configurations by updating Playwright test identifiers for better clarity and consistency across different project states. - Enhanced cleanup functions in tests to handle directory removal more robustly, preventing errors during test execution. * fix: Resolve React hooks errors from duplicate instances in dependency tree * style: Format alias configuration for improved readability --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: DhanushSantosh <dhanushsantoshs05@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
useIsMobile()hook to both MemoryView and ContextView components for responsive behaviorgap-1) for compact layoutisMarkdownFile,isImageFile) to module-level constants for code reuse and consistencydata-testidattribute to HeaderActionsPanelTrigger for test coverageSummary by CodeRabbit
New Features
Tests
Chores