Skip to content

Preserve feature namespaces and constrain execution drawer width - #1125

Merged
sfmskywalker merged 1 commit into
mainfrom
codex/studio-review-followups
Oct 5, 2026
Merged

sfmskywalker merged 1 commit into
mainfrom
codex/studio-review-followups

Conversation

@sfmskywalker

@sfmskywalker sfmskywalker commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Follow-up validation of the consolidated import found two Studio edge cases. Remote feature fallback could enable an Acme-qualified request from an Elsa catalog entry sharing its final name, and the 600px execution drawer could extend beyond a narrow viewport.

Require an Elsa-qualified request for the Elsa short-name fallback while preserving exact foreign-name matching. Add feature initialization regression cases for both catalog representations and exact foreign matches. Cap the drawer at 100vw while keeping its 600px desktop width and existing dismissal behavior.

The fixes are also applied in Core #8624. Hosted CI and independent exact-head review will qualify this source follow-up. Merge with a merge commit and a [skip ci] merge subject to avoid automatic package publication.

Refs elsa-workflows/elsa-core#8623, elsa-workflows/elsa-core#8624.

Summary by CodeRabbit

  • Bug Fixes
    • The activity execution details drawer now fits within the viewport, improving access to its contents on narrow screens.
    • Remote feature requests now distinguish between Elsa-qualified requests and requests for other features, preventing unrelated requests from initializing an Elsa feature. Exact feature-name matches continue to be recognized.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d3c5f70a-e25d-4ae6-9d83-b049ed042510
📥 Commits

Reviewing files that changed from the base of the PR and between e712b5e and 734da58.

📒 Files selected for processing (3)
  • src/framework/Elsa.Studio.Core.Tests/DefaultFeatureServiceTests.cs
  • src/framework/Elsa.Studio.Core/Services/RemoteFeatureCatalog.cs
  • src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/ActivityExecutionDetailsDrawer.razor

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The changes restrict remote-feature fallback matching to Elsa-qualified requests and add tests for Elsa and Acme request names. The activity execution details drawer now has a maximum width of 100vw.

Changes

Remote feature matching

Layer / File(s) Summary
Catalog matching and tests
src/framework/Elsa.Studio.Core/Services/RemoteFeatureCatalog.cs, src/framework/Elsa.Studio.Core.Tests/DefaultFeatureServiceTests.cs
Matches checks exact names first. It applies short-name and feature-ID fallbacks only to Elsa-qualified requests. Tests cover Elsa and Acme request shapes.

Activity details drawer sizing

Layer / File(s) Summary
Set drawer maximum width
src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/ActivityExecutionDetailsDrawer.razor
The drawer style sets max-width: 100vw.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: cursoragent

Merge Risk: ⚪ Minimal · up to 734da

No actionable merge risk is established by the supplied change context.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 734da

The change tightens feature matching without granting additional access: foreign feature requests now require an exact catalog match, while existing Elsa aliases remain supported. The drawer adjustment does not change execution-data access or dismissal controls. No introduced or worsened security concern was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated impact is on Studio feature eligibility and enabled-feature results for the supplied catalog. The changed predicate introduces no additional input source, credential authority, or execution sink.

Security Findings and Attack Paths

  • inferred — For identical request and catalog inputs, the new rejection branch cannot create a successful match that the base rejected. It removes the demonstrated namespace-collision route rather than expanding feature activation. This conclusion does not establish backend authorization correctness.

Trust Boundaries and Controls

  • observed — The unchanged provider checks authentication when an AuthenticationStateProvider is supplied and returns an empty catalog for unauthenticated users or unauthorized/forbidden responses. Successful catalog caching remains backend-URL-bound. Feature-name matching does not add a server-side authorization control.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes both main changes: preserving feature namespaces and constraining the execution drawer width.
Description check ✅ Passed The description explains the two problems, the proposed fixes, and the related Core changes. It is mostly complete, but it does not use the template sections or provide concrete verification steps and…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Feature matching logic and UI drawer styling.

No outstanding findings block merging.

Summary

The PR limits Elsa short-name feature fallback to Elsa-qualified requests while retaining exact matches for foreign names, adds initialization regression cases, and caps the execution-details drawer at the viewport width. Users can open execution details on a narrow screen without the drawer extending past the viewport.

Reviews (2) · Last reviewed commit: "Restrict feature fallback and constrain ..."

@sfmskywalker

Copy link
Copy Markdown
Member Author

@greptileai The non-blocking viewport-automation suggestion has a direct disposition reply and the thread is resolved. Existing component interaction tests plus the successful Chromium viewport bounds check qualify the small CSS cap; a bUnit style-string assertion would merely mirror the implementation. Please reassess the same head with that scope disposition.

@greptile-apps

greptile-apps Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

This PR is set for a Plus review. TREX isn't supported with Plus or Apex reviews yet, so Greptile reviews this PR at Base while TREX is on.

@sfmskywalker sfmskywalker left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Elsa 3 Code Review: APPROVE + HIGH @ 734da58

Independent read-only review finds no blocking defect. Exact foreign full-name matches remain supported; only Elsa-qualified requests can use short-name fallback. Three regression cases cover foreign false aliases and exact matching. The drawer width cap preserves its desktop width and existing interaction behavior.

Exact-head CI https://github.com/elsa-workflows/elsa-studio/actions/runs/37386031336 passed: build succeeded with zero errors; all 18 test results total 1,911 passed, zero failed/skipped. CodeQL, GitGuardian, CLA and CodeRabbit are green.

Greptile's published score is 4/5 with one explicitly non-blocking request for a dedicated viewport browser test; a same-head re-score is running. Its executed Chromium comparison confirms the capped drawer fits a 375px viewport without document overflow. The disposition is proportionate: existing component interaction coverage is retained, while a bUnit literal-style assertion would merely mirror the CSS. Studio's current reviewer policy makes Greptile advisory.

Posted by the integrating lead on behalf of the independent workroom QA agent, who made no implementation changes.

@sfmskywalker
sfmskywalker merged commit 3d028a4 into main Oct 5, 2026
11 checks passed
@sfmskywalker
sfmskywalker deleted the codex/studio-review-followups branch October 5, 2026 23:14
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.

1 participant