Skip to content

fix: include back notifications widget - #2499

Merged
benoitf merged 1 commit into
openkaiden:mainfrom
benoitf:KAIDEN-2497
Jul 21, 2026
Merged

benoitf merged 1 commit into
openkaiden:mainfrom
benoitf:KAIDEN-2497

Conversation

@benoitf

@benoitf benoitf commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

fixes #2497

fixes #2497

Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@benoitf
benoitf requested a review from a team as a code owner July 20, 2026 15:42
@benoitf
benoitf requested review from MarsKubeX and jeffmaury and removed request for a team July 20, 2026 15:42
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 56498fdd-2b85-4c3e-bf9f-be81a9355a29

📥 Commits

Reviewing files that changed from the base of the PR and between 699463a and fd535d4.

📒 Files selected for processing (2)
  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts
  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte
📜 Recent review details
⏰ Context from checks skipped due to timeout. (10)
  • GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
  • GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
  • GitHub Check: Windows
  • GitHub Check: unit tests / macos-15
  • GitHub Check: unit tests / ubuntu-24.04
  • GitHub Check: Linux
  • GitHub Check: linter, formatters
  • GitHub Check: unit tests / windows-2022
  • GitHub Check: typecheck
  • GitHub Check: macOS
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use /@/ path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory

Files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts
**/*.spec.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.spec.{ts,tsx,js,jsx}: Use test() instead of it() for test cases in Vitest unit tests
Use vi.mock(import('...')) for auto-mocking modules in unit tests; avoid manual mock factories when possible
Use vi.resetAllMocks() in beforeEach hooks instead of vi.clearAllMocks() for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, use vi.mocked(...) with the prototype pattern for class methods: vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)

Files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Container operations must include engineId parameter to identify the container engine

Files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts
🧠 Learnings (11)
📚 Learning: 2026-03-17T11:49:39.964Z
Learnt from: MarsKubeX
Repo: kortex-hub/kortex PR: 1111
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceCard.svelte:46-52
Timestamp: 2026-03-17T11:49:39.964Z
Learning: In Svelte components like AgentWorkspaceCard.svelte and CustomPick.svelte, using a div with role="button" and tabindex="0" as the clickable card container with an inner native <button> (e.g., for a remove action) is acceptable because nested <button> elements are invalid per HTML spec. Ensure the inner button's events do not bubble by calling stopPropagation on both click and keydown handlers. Do not flag this pattern as an accessibility issue when implemented this way, but verify that keyboard activation (Enter/Space) and ARIA semantics are preserved and that focus management remains clear.

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte
📚 Learning: 2026-04-15T08:04:32.031Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.svelte:9-11
Timestamp: 2026-04-15T08:04:32.031Z
Learning: For Svelte components in this repo, if a callback prop is typed as `() => void`, TypeScript idiomatically allows passing async functions (e.g., `() => Promise<void>`), because `() => void` indicates the caller ignores the return value rather than requiring `undefined`. Do not recommend changing these prop types to `() => void | Promise<void>` solely to “fix” async compatibility—unless there is an actual need for the caller to observe the returned value.

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte
📚 Learning: 2026-04-17T08:04:22.761Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 1362
File: packages/renderer/src/lib/secret-vault/SecretVaultList.svelte:23-29
Timestamp: 2026-04-17T08:04:22.761Z
Learning: In packages/renderer, the established list-page convention for Svelte components (e.g., *SkillsList.svelte, *SecretVaultList.svelte) is to use Svelte `$effect` blocks to sync local `$state` variables (like `searchTerm` or `screen`) into their corresponding Svelte stores (e.g., `*SearchPattern`, `*CategoryFilter`). During code review, do not treat this `$effect`-based state→store synchronization as an anti-pattern or recommend replacing it with direct store bindings or event-handler-only updates; it is intentional and consistent across these list pages.

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte
📚 Learning: 2026-04-28T13:34:51.610Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1431
File: packages/renderer/src/lib/guided-setup/panels/OpenCodePanel.svelte:14-20
Timestamp: 2026-04-28T13:34:51.610Z
Learning: In this repo’s Svelte renderer (packages/renderer/src/**/*.svelte), `podman-desktop/ui-svelte`’s `Link` component does not accept an `href` prop. For opening external URLs, use the established pattern `on:click={() => window.openExternal(url)}` (optionally typed as `on:click={(): Promise<void> => window.openExternal(url)}`), consistent with existing components like `ProviderLinks.svelte`, `WelcomePage.svelte`, and `OpenCodePanel.svelte`. Do not treat `window.openExternal()` usage in renderer Svelte components as bypassing a security restriction, and do not recommend replacing it with `Link`/`href`-based navigation. (There is also no `setupSecurityRestrictionsOnLinks` utility in the renderer.)

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte
📚 Learning: 2026-04-29T11:54:22.423Z
Learnt from: vancura
Repo: openkaiden/kaiden PR: 1494
File: packages/renderer/src/Loader.svelte:13-13
Timestamp: 2026-04-29T11:54:22.423Z
Learning: In this codebase’s renderer, timer variables in Svelte files (e.g., in `packages/renderer/src/Loader.svelte`) that store `setTimeout` handles are intentionally typed as `NodeJS.Timeout` (and not `ReturnType<typeof setTimeout>`). When reviewing, do not flag or recommend changing this to `ReturnType<typeof setTimeout>` for portability—treat `NodeJS.Timeout` as the established convention for renderer timer variables.

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte
📚 Learning: 2026-07-14T11:55:38.534Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 2454
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceTerminal.svelte:143-160
Timestamp: 2026-07-14T11:55:38.534Z
Learning: In this Electron app, the main process prevents renderer `beforeunload` from firing on window close/app quit/navigation by calling `e.preventDefault()` on `browserWindow` `close` and then either `hide()` (no unload) or `destroy()` (bypasses `beforeunload`), and `app.on('before-quit')` also calls `browserWindow.destroy()`. As a result, renderer `beforeunload` (and any “reload confirmation” handler) should only run for an explicit page reload (e.g., `location.reload()`) and not for close/quit actions. During review, do NOT flag hardcoded `location.reload()` calls inside `beforeunload`/reload-confirmation handlers as “hijacking” close/quit behavior.

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte
  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts
📚 Learning: 2026-06-10T15:22:39.639Z
Learnt from: bmahabirbu
Repo: openkaiden/kaiden PR: 2115
File: packages/renderer/src/lib/models/SemanticRouterCreate.svelte:134-137
Timestamp: 2026-06-10T15:22:39.639Z
Learning: In multi-step wizard UIs, it’s acceptable for “Next step” buttons to be temporarily non-interactive during incremental work delivered across multiple PRs. When reviewing Svelte files under `packages/renderer/src/lib/**`, do not flag these UX issues if the button lacks an onClick handler *and* there is an explicit TODO comment indicating deferred wiring, and the PR author confirms (in the PR description) that this is an intentional staged implementation. If no TODO/deferred-wiring marker (or no author confirmation) is present, treat missing handlers as a potential UX issue.

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte
📚 Learning: 2026-04-15T08:51:08.199Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1336
File: packages/renderer/src/lib/guided-setup/GuidedSetup.spec.ts:64-69
Timestamp: 2026-04-15T08:51:08.199Z
Learning: For Svelte component test files under `packages/renderer/src/**` with names ending in `.spec.ts` that use `render` from `testing-library/svelte`, call `vi.useFakeTimers({ shouldAdvanceTime: true })` inside a `beforeEach`. Keep it in `beforeEach` for these tests to avoid flakiness from Svelte’s internal tick scheduling; do not remove or flag it based on the presence/absence of explicit timer manipulation in individual tests.

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts
📚 Learning: 2026-06-16T06:15:26.225Z
Learnt from: gastoner
Repo: openkaiden/kaiden PR: 2145
File: packages/renderer/src/lib/mcp/MCPServerRemoteListActions.spec.ts:46-49
Timestamp: 2026-06-16T06:15:26.225Z
Learning: For renderer unit tests in this repo (files included in `packages/renderer/vite.config.js` via `setupFiles`), do not manually mock `window` API methods in `beforeEach` or within the spec. The test setup (`packages/renderer/vite.tests.setup.js`) reads `packages/preload/exposedInMainWorld.d.ts` and defines each declared `Window` method on `window` as a `vi.fn()` using `Object.defineProperty`, so `window.*` methods (e.g., `startMcpServer`, `stopMcpServer`, `showMessageBox`) are already present as mocks. Only add an explicit mock if the method is not declared in `exposedInMainWorld.d.ts` (in which case update the `.d.ts` so the shared setup can generate the mock).

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts
📚 Learning: 2026-06-29T13:16:53.102Z
Learnt from: benoitf
Repo: openkaiden/kaiden PR: 2296
File: extensions/container/packages/extension/src/helper/socket-finder/_socket-finder-module.ts:28-29
Timestamp: 2026-06-29T13:16:53.102Z
Learning: When reviewing imports in openkaiden/kaiden TypeScript/JavaScript files, prefer the configured `/@/` path alias instead of relative imports that would require traversing out of the current directory/module group (i.e., paths containing `..` that cross boundaries). 

Do not require alias conversion for descendant-path relative imports within the socket-finder module directory—for example, in `extensions/container/packages/extension/src/helper/socket-finder/**`, imports like `./podman/podman-version-detector` and `./podman/podman-windows-finder` are acceptable and should not be flagged.

Applied to files:

  • packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts
🔇 Additional comments (2)
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte (1)

14-14: LGTM!

Also applies to: 104-104

packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts (1)

24-34: LGTM!

Also applies to: 108-131


📝 Walkthrough

Walkthrough

AgentWorkspaceList now renders NotificationsBox in its NavPage content. Tests reset notificationQueue and verify that the box is hidden when empty and visible when it contains a highlighted notification.

Changes

Agent workspace notifications

Layer / File(s) Summary
NotificationsBox integration and coverage
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte, packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.spec.ts
AgentWorkspaceList renders NotificationsBox, and tests cover its visibility for empty and populated notification queues.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • openkaiden/kaiden#2475: Updates notification forwarding that populates the queue consumed by NotificationsBox.

Suggested reviewers: bmahabirbu, jeffmaury, marskubex

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: adding the notifications widget to the landing page.
Description check ✅ Passed The description references the linked issue and aligns with the notifications visibility fix.
Linked Issues check ✅ Passed The PR adds NotificationsBox to AgentWorkspaceList, matching #2497's requirement to display notifications on the main landing page.
Out of Scope Changes check ✅ Passed The changes are limited to the notifications widget and related tests, with no clear unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@fbricon

fbricon commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

what kind of notifications should we expect?

@benoitf

benoitf commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@fbricon all notifications added by calling notificationRegistry.addNotification

that we usually automatically see as it's included in the default landing page
https://github.com/openkaiden/kaiden/blob/main/packages/renderer/src/lib/dashboard/DashboardPage.svelte#L180

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@benoitf

benoitf commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

example of notification: #2500

@benoitf
benoitf merged commit 7d7658b into openkaiden:main Jul 21, 2026
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notifications are not visible in Kaiden

4 participants