Skip to content

fix(web): clear deleted skill context from composer (#2637) - #7137

Open
T-sanjay-ram wants to merge 2 commits into
nexu-io:mainfrom
T-sanjay-ram:fix/clear-deleted-skill-context-2637
Open

fix(web): clear deleted skill context from composer (#2637)#7137
T-sanjay-ram wants to merge 2 commits into
nexu-io:mainfrom
T-sanjay-ram:fix/clear-deleted-skill-context-2637

Conversation

@T-sanjay-ram

@T-sanjay-ram T-sanjay-ram commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #2637.

Why

Deleting a user skill refreshes the available skills catalog, but a composer that had already staged that skill could keep its stale context chip and inline mention. That leaves the composer context row and draft out of sync and can cause the reported layout break.

What users will see

If you delete a skill that is currently staged in the composer, its context chip and matching inline @skill mention are cleared together. Other draft text and context remain 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 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 (see TRANSLATIONS.md for the locale workflow)
  • New top-level dependency — adding any new entry to the root package.json (dependencies or devDependencies); workspace-package package.json files are out of scope
  • Default behavior change — changes what existing users experience without opting in (default model, default setting, file/SQLite schema, auto-network on startup, auto-install)
  • None — internal refactor, docs, tests, or translation update only

Screenshots

Not applicable: this fixes stale state in an existing composer flow and adds no new visual entry point.

Bug fix verification

  • Test path that reproduces the bug: apps/web/tests/components/ChatComposer.context-pickers.test.tsx
  • Did the test go red on main and green on this branch? Not run locally: this machine's repository checkout stalled during download. The regression covers staging @Deck Builder, refreshing the supplied catalog without it, and verifying the staged context row and inline mention are cleared.

Validation

  • Local test execution was unavailable because this machine's repository checkout stalled during download.
  • GitHub Actions CI is running.

@lefarcen

Copy link
Copy Markdown
Contributor

Thanks @T-sanjay-ram — this looks like a nicely scoped fix for keeping the composer context strip and inline mention state aligned after a deleted skill disappears from the catalog. I’m routing the PR and the required checks now.

@lefarcen
lefarcen requested a review from PerishCode August 19, 2026 15:29
@lefarcen lefarcen added size/S PR changes 20-100 lines risk/medium Medium risk: regular code changes type/bugfix Bug fix labels Aug 19, 2026

@lefarcen lefarcen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @T-sanjay-ram — the regression write-up is clear and the validation context helps. Before pool review picks this up, could you add a short user-facing "what changes in the composer after deleting a skill" note and fill in the Surface area checklist so reviewers can scope the impact quickly?

@lefarcen

Copy link
Copy Markdown
Contributor

🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we'll loop QA in once it's merge-ready (and design/product have signed off, where applicable).

@lefarcen lefarcen added the needs-validation Runtime change detected; needs human or /explore agent validation. label Aug 19, 2026

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@T-sanjay-ram This keeps the composer’s staged skill chip and atomic inline mention synchronized when an authoritative refreshed catalog removes that skill, while preserving the omitted-catalog compatibility path. I verified the catalog lifecycle and draft-ref mutation paths, and the focused regression test, web typecheck, and repository guard all pass. Nicely scoped fix and solid regression coverage—thank you for closing this stale-context edge case.

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

@lefarcen
lefarcen requested a review from AmyShang-alt August 19, 2026 15:44
@github-actions

Copy link
Copy Markdown
Contributor

Visual regression review

Head: 83e5c46 · Base: 057b0f4

⚠️ 1 case(s) failed during diff generation; partial captures are shown below.

0 changed · 21 unchanged · 0 new without baseline · 1 failed

Capture or diff failures

Unchanged cases
Case Main PR Diff
visual-design-system-detail
0 px (0.00%)
main pr diff
visual-design-systems
0 px (0.00%)
main pr diff
visual-home
0 px (0.00%)
main pr diff
visual-home-catalog
0 px (0.00%)
main pr diff
visual-home-context-picker
0 px (0.00%)
main pr diff
visual-home-context-picker-popover
0 px (0.00%)
main pr diff
visual-home-plugin-filter
0 px (0.00%)
main pr diff
visual-home-plugin-use-staged
0 px (0.00%)
main pr diff
visual-home-plugin-use-with-query
0 px (0.00%)
main pr diff
visual-home-staged-attachment
0 px (0.00%)
main pr diff
visual-integrations
0 px (0.00%)
main pr diff
visual-integrations-mcp
0 px (0.00%)
main pr diff
visual-integrations-use-everywhere
0 px (0.00%)
main pr diff
visual-new-project-modal
0 px (0.00%)
main pr diff
visual-onboarding-cloud
0 px (0.00%)
main pr diff
visual-plugin-details
0 px (0.00%)
main pr diff
visual-plugin-share-menu
0 px (0.00%)
main pr diff
visual-plugin-share-menu-popover
0 px (0.00%)
main pr diff
visual-plugins
0 px (0.00%)
main pr diff
visual-projects
0 px (0.00%)
main pr diff

Visual diff is advisory only and does not block merging.

Copy link
Copy Markdown
Contributor Author

CI note: Validate workspace is red because it treats the required UI P0 and visual jobs as non-successful.

Those jobs were cancelled during Playwright setup—specifically while pnpm -C e2e exec playwright install --with-deps chromium was installing system dependencies—and hit the 30-minute limit before any test assertions or visual diff checks ran. The completed test, typecheck, and guard jobs passed; the visual report also shows 0 changed and 21 unchanged captures.

Could a maintainer rerun the cancelled CI jobs when capacity permits?

@lefarcen

Copy link
Copy Markdown
Contributor

Thanks for digging into the failing Validate workspace status and calling out that the red check comes from cancelled Playwright setup rather than a failing assertion. I’ve passed the rerun request along so a maintainer can retry those cancelled UI P0 / visual jobs when capacity opens up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-validation Runtime change detected; needs human or /explore agent validation. risk/medium Medium risk: regular code changes size/S PR changes 20-100 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deleting a skill breaks the dialog layout

3 participants