Skip to content

Do not leak initially visible action sets into perspectives - #4391

Open
akurtakov wants to merge 1 commit into
eclipse-platform:masterfrom
akurtakov:actionset-leak-into-model-perspectives
Open

akurtakov wants to merge 1 commit into
eclipse-platform:masterfrom
akurtakov:actionset-leak-into-model-perspectives

Conversation

@akurtakov

@akurtakov akurtakov commented Sep 15, 2026

Copy link
Copy Markdown
Member

extendPerspectives() reads the perspectiveExtensions contributions into a
scratch perspective and copies the resulting action set tags onto the real
one. Constructing its ModeledPageLayout already tags that scratch perspective
with every action set declared visible="true", so those were copied as well.

This surfaced once model contributed perspectives became known to the
PerspectiveRegistry and thus reached this code: a pure e4 application got the
legacy action sets of every bundle it ships, appended to its trim as disabled
tool items.

Drop the seeded tags before reading the extensions so that only what the
extension point contributes is taken over.

Assisted-by: Anthropic Claude Code (claude-opus-5[1m])

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unique model tags can cause valid extension-contributed action sets to be omitted.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Prevents initially visible legacy action sets from leaking into model-contributed perspectives.

Changes:

  • Tracks tags added by perspective extensions.
  • Adds regression coverage for model perspectives.
  • Registers the test in the API suite.
File summaries
File Description
WorkbenchPage.java Filters constructor-added action-set tags.
ModelPerspectiveActionSetTest.java Tests action-set leakage.
ApiTestSuite.java Adds the regression test.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Test Results

   861 files  ±0     861 suites  ±0   50m 19s ⏱️ -4s
 8 337 tests +1   8 094 ✅ +1  243 💤 ±0  0 ❌ ±0 
20 880 runs  +3  20 210 ✅ +3  670 💤 ±0  0 ❌ ±0 

Results for commit 7e5d528. ± Comparison against base commit a6b4946.

♻️ This comment has been updated with latest results.

@akurtakov
akurtakov force-pushed the actionset-leak-into-model-perspectives branch from 4d29bd6 to 238fe12 Compare September 15, 2026 19:03
@akurtakov
akurtakov requested a balanced review from Copilot September 15, 2026 19:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused fix correctly preserves explicit contributions while the regression test covers the reported leakage.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@akurtakov
akurtakov force-pushed the actionset-leak-into-model-perspectives branch 2 times, most recently from c81f769 to 52f0561 Compare September 15, 2026 19:21
extendPerspectives() reads the perspectiveExtensions contributions into a
scratch perspective and copies the resulting action set tags onto the real
one. Constructing its ModeledPageLayout already tags that scratch perspective
with every action set declared visible="true", so those were copied as well.

This surfaced once model contributed perspectives became known to the
PerspectiveRegistry and thus reached this code: a pure e4 application got the
legacy action sets of every bundle it ships, appended to its trim as disabled
tool items.

Drop the seeded tags before reading the extensions so that only what the
extension point contributes is taken over.

Assisted-by: Anthropic Claude Code (claude-opus-5[1m])
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.

2 participants