Skip to content

feat(knowledge): persist Cosmos DB resource ID - #9598

Open
Bryan Bennett (bjbennet) wants to merge 7 commits into
Azure:mainfrom
bjbennet:brbenn-microsoft-cosmos-resource-id
Open

Bryan Bennett (bjbennet) wants to merge 7 commits into
Azure:mainfrom
bjbennet:brbenn-microsoft-cosmos-resource-id

Conversation

@bjbennet

Copy link
Copy Markdown
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Knowledge hub connections currently persist the selected Cosmos DB endpoint but discard the Azure resource ID chosen in the connection wizard. This change serializes the existing Cosmos DB account selection as cosmosDB.resourceId, making the account identity available alongside the endpoint in connections.json.

Impact of Change

  • Users: Newly created and updated knowledge hub connections retain the selected Cosmos DB account resource ID.
  • Developers: KnowledgeHubConnectionModel.cosmosDB now includes a required resourceId property.
  • System: The Standard connection serializer writes the selected account ID under knowledgeHubConnections.HubConnection.cosmosDB.resourceId and restores it when editing.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: Focused Vitest suites for designer knowledge connection metadata and Standard connection persistence (43 tests)

Contributors

@brbenn_microsoft

Screenshots/Videos

N/A - no visual changes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 3, 2026 17:42
@bjbennet Bryan Bennett (bjbennet) added the risk:medium Medium risk change with potential impact label Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: feat(knowledge): persist Cosmos DB resource ID
  • Issue: None — valid feat: prefix with scope and a descriptive summary.
  • Recommendation: No change needed.

Commit Type

  • feat is selected and matches the title prefix.
  • Exactly one type selected — good.

Risk Level

  • Declared Medium (body) and label risk:medium are consistent, and this matches the advised estimate. The change touches shared runtime code (libs/logic-apps-shared), extension distribution (apps/vs-code-designer), and designer state/UI — squarely Medium.

What & Why

  • Current: Clear explanation that Cosmos DB resource ID was previously discarded and is now serialized as cosmosDB.resourceId.
  • Issue: None.
  • Recommendation: No change needed.

Impact of Change

  • All three audiences (Users, Developers, System) are addressed with specifics.
  • Recommendation:
    • Users: Adequately described (connections retain selected Cosmos DB account ID).
    • Developers: Adequately described (KnowledgeHubConnectionModel.cosmosDB.resourceId).
    • System: Adequately described (Standard serializer writes/restores resource ID).

Test Plan

  • Unit tests added/updated is checked and confirmed across the diff (designer, designer-v2, vs-code-designer, and logic-apps-shared standard connection suites). Requirement satisfied.

Contributors

  • @brbenn_microsoft credited. Thanks for acknowledging collaborators.

⚠️ Screenshots/Videos

  • The diff touches libs/designer/src/lib/ui/knowledge/panel/connection/edit.tsx, but the change is behavioral (resource-ID resolution, save-button enablement) rather than a visual/layout change, and it's marked N/A. No screenshot required — just confirm there is no visible UI difference.

Summary Table

Section Status Recommendation
Title No change needed
Commit Type No change needed
Risk Level Medium is correct
What & Why No change needed
Impact of Change No change needed
Test Plan Unit tests confirmed
Contributors No change needed
Screenshots/Videos ⚠️ Confirm no visual change; N/A acceptable

All required checks pass — this PR is compliant and cleared to merge. Nice, thorough test coverage.


Powered by: Copilot CLI (claude-opus-4.8) | Last updated: Tue, 15 Sep 2026 19:12:56 GMT

Copilot AI 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.

🟡 Changes recommended

Existing connections cannot persist updates, and requiring resourceId breaks compatibility with legacy connection data.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Persists Cosmos DB resource IDs for Knowledge Hub connections and restores them during editing.

Changes:

  • Adds cosmosDB.resourceId serialization and restoration.
  • Extends connection models and focused unit coverage.
  • Requires fixes for legacy compatibility and updating existing connections.
File summaries
File Review
libs/logic-apps-shared/src/designer-client-services/lib/standard/connection.ts Adds resourceId, but it must remain optional for legacy data compatibility.
libs/logic-apps-shared/src/designer-client-services/lib/standard/__tests__/connection.spec.ts Tests resource ID serialization.
libs/designer/src/lib/core/knowledge/utils/connection.ts Adds serialization, but existing connections are not overwritten by host writers.
libs/designer/src/lib/core/knowledge/utils/__test__/connection.spec.ts Tests metadata and edit restoration; host-writer update coverage is still needed.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread libs/designer/src/lib/core/knowledge/utils/connection.ts
Comment thread libs/logic-apps-shared/src/designer-client-services/lib/standard/connection.ts Outdated
Comment thread apps/vs-code-react/src/app/designer/servicesHelper.ts
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:medium Medium risk change with potential impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants