Skip to content

fix: confirm extraction history item deletion#2261

Open
itsbryanman wants to merge 1 commit into
nexu-io:mainfrom
itsbryanman:fix/extraction-history-delete-confirmation-2249
Open

fix: confirm extraction history item deletion#2261
itsbryanman wants to merge 1 commit into
nexu-io:mainfrom
itsbryanman:fix/extraction-history-delete-confirmation-2249

Conversation

@itsbryanman
Copy link
Copy Markdown

Fixes #2249

Why

Deleting an individual extraction history item in Settings → Memory → Extraction history happened immediately after clicking the row delete control.

That made it too easy for a user to accidentally remove their own extraction history data with no confirmation, warning, or undo path. This PR adds a confirmation step for the individual row delete flow so the delete action only runs after explicit confirmation.

What users will see

In Settings → Memory → Extraction history, clicking the delete control on a single history item now opens a confirmation dialog.

The item is not deleted unless the user confirms the action. Canceling or dismissing the dialog leaves the history item unchanged.

Surface area

  • UI — new page / dialog / panel / menu item / setting / empty state in apps/web or apps/desktop (including Electron menu bar)
  • Keyboard shortcut — new or changed
  • CLI / env var — new od subcommand or flag, new tools-dev / tools-pack / tools-pr flag, or new OD_* env var
  • API / contract — new /api/* endpoint, new SSE event, or changed shape in packages/contracts
  • Extension point — new entry under skills/, design-systems/, design-templates/, or craft/, or change to the skills protocol
  • i18n keys — added new translation keys
  • New top-level dependency — adding any new entry to the root package.json
  • Default behavior change — changes what existing users experience without opting in
  • None — internal refactor, docs, tests, or translation update only

Screenshots

Not attached.

The visible change is a confirmation dialog shown after clicking the delete control for an individual extraction history item.

Bug fix verification

  • Test path that reproduces the bug:

    • apps/web/tests/components/MemorySection.test.tsx
  • Added/updated tests covering:

    • Clicking the individual row delete control does not immediately call the delete handler/API.
    • The confirmation dialog appears after clicking delete.
    • Canceling the dialog does not delete the item.
    • Confirming the dialog deletes only the selected extraction history item.
    • Failed deletion keeps/restores the item instead of silently removing it.
  • Did the test go red on main and green on this branch?

    • Not separately verified on main.
    • The updated targeted test suite passes on this branch.

Validation

Passed locally:

  • ./node_modules/.bin/vitest run -c vitest.config.ts tests/components/MemorySection.test.tsx

    • Result: 20 passed
  • ./node_modules/.bin/tsc -b --noEmit

    • Result: passed

Lint:

  • apps/web doesnt define a lint script

@lefarcen lefarcen requested a review from nettee May 19, 2026 11:38
@lefarcen lefarcen added size/M PR changes 100-300 lines risk/medium Medium risk: regular code changes type/bugfix Bug fix labels May 19, 2026
Copy link
Copy Markdown
Contributor

@nettee nettee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one blocking issue in the updated test coverage and one follow-up i18n concern.

🔁 Powered by Looper · runner=reviewer · agent=opencode · model=openai/gpt-5.4 · An autonomous AI dev team for your GitHub repos.

Comment thread apps/web/tests/components/MemorySection.test.tsx Outdated
Comment thread apps/web/src/components/MemorySection.tsx Outdated
@itsbryanman itsbryanman force-pushed the fix/extraction-history-delete-confirmation-2249 branch from 1a61c0b to 587f5dc Compare May 19, 2026 12:20
@lefarcen lefarcen requested a review from nettee May 19, 2026 12:27
Copy link
Copy Markdown
Contributor

@nettee nettee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one blocking regression in the new confirmation flow and one non-blocking localization follow-up.

🔁 Powered by Looper · runner=reviewer · agent=opencode · model=openai/gpt-5.4 · An autonomous AI dev team for your GitHub repos.

Comment thread apps/web/src/i18n/locales/en.ts Outdated
Comment thread apps/web/src/components/MemorySection.tsx Outdated
@itsbryanman itsbryanman force-pushed the fix/extraction-history-delete-confirmation-2249 branch from 587f5dc to 6b22b15 Compare May 19, 2026 12:51
@lefarcen lefarcen requested a review from nettee May 19, 2026 12:59
Copy link
Copy Markdown
Contributor

@nettee nettee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one blocking regression in the new confirmation flow.

🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.

Comment thread apps/web/src/components/MemorySection.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/medium Medium risk: regular code changes size/M PR changes 100-300 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deleting an extraction history item does not require confirmation

3 participants