Skip to content

Export terminal chat context key strings to prevent hardcoded strings and layering violations#283907

Merged
meganrogge merged 2 commits intomainfrom
copilot/ensure-context-keys-enum
Dec 16, 2025
Merged

Export terminal chat context key strings to prevent hardcoded strings and layering violations#283907
meganrogge merged 2 commits intomainfrom
copilot/ensure-context-keys-enum

Conversation

Copy link
Contributor

Copilot AI commented Dec 16, 2025

Context key strings for terminal chat features were hardcoded as strings ('hasHiddenChatTerminals', 'hasChatTerminals') instead of referencing the TerminalChatContextKeyStrings enum. Direct imports from terminalContrib/chat caused layering violations.

Changes

Exported context key strings via terminalContribExports.ts

  • Added TerminalContribContextKeyStrings enum exporting ChatHasTerminals and ChatHasHiddenTerminals
  • Follows existing pattern for TerminalContribCommandId and TerminalContribSettingId

Replaced hardcoded strings in three files:

  • terminalMenus.ts
  • terminalTabbedView.ts
  • terminalContextKey.ts
// Before
ContextKeyExpr.not('hasHiddenChatTerminals')

// After
ContextKeyExpr.not(TerminalContribContextKeyStrings.ChatHasHiddenTerminals)

This provides type safety, prevents typos, and maintains proper layer separation between terminal/ and terminalContrib/.

Original prompt

This section details on the original issue you should resolve

<issue_title>Ensure context keys are part of an enum</issue_title>
<issue_description>From https://github.com/microsoft/vscode/pull/277452/files

Image

if that is the reason we could have prevented this by using a const enum like TerminalContextKeyStrings</issue_description>

Comments on the Issue (you are @copilot in this section)

@meganrogge So they are a part of that enum, but importing the enum here results in a layering warning .

Image</comment_new>
<comment_new>@Tyriar
If they're used outside terminalContrib/chat they should live in terminalContribExports.ts</comment_new>


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com>
Copilot AI changed the title [WIP] Ensure context keys are part of an enum Export terminal chat context key strings to prevent hardcoded strings and layering violations Dec 16, 2025
Copilot AI requested a review from meganrogge December 16, 2025 21:06
@meganrogge meganrogge added this to the December / January 2026 milestone Dec 16, 2025
@meganrogge meganrogge marked this pull request as ready for review December 16, 2025 21:42
Copilot AI review requested due to automatic review settings December 16, 2025 21:42
@meganrogge meganrogge enabled auto-merge (squash) December 16, 2025 21:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a layering violation issue by properly exporting terminal chat context key strings through the terminalContribExports.ts bridge file. Previously, code in the terminal/ layer was using hardcoded string literals ('hasHiddenChatTerminals', 'hasChatTerminals') instead of referencing the enum values from TerminalChatContextKeyStrings in terminalContrib/chat. Direct imports would have violated the layering architecture, so the solution follows the existing pattern of re-exporting these values through terminalContribExports.ts.

Key changes:

  • Added TerminalContribContextKeyStrings enum to terminalContribExports.ts that re-exports ChatHasTerminals and ChatHasHiddenTerminals
  • Replaced hardcoded strings with enum references in terminalContextKey.ts, terminalTabbedView.ts, and terminalMenus.ts

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/vs/workbench/contrib/terminal/terminalContribExports.ts Added TerminalContribContextKeyStrings enum exporting chat context keys to provide layer-safe access
src/vs/workbench/contrib/terminal/common/terminalContextKey.ts Replaced hardcoded 'hasHiddenChatTerminals' string with enum reference in shouldShowViewInlineActions expression
src/vs/workbench/contrib/terminal/browser/terminalTabbedView.ts Replaced hardcoded 'hasHiddenChatTerminals' string with enum reference in context change listener
src/vs/workbench/contrib/terminal/browser/terminalMenus.ts Replaced hardcoded 'hasHiddenChatTerminals' string with enum reference in menu visibility condition

@meganrogge meganrogge merged commit a32f7d1 into main Dec 16, 2025
33 of 34 checks passed
@meganrogge meganrogge deleted the copilot/ensure-context-keys-enum branch December 16, 2025 22:29
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Jan 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ensure context keys are part of an enum

4 participants