Skip to content

fix(vscode): Fix workflow save token persistence and parameter merge#9415

Draft
bjbennet with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-code-for-review-comments
Draft

fix(vscode): Fix workflow save token persistence and parameter merge#9415
bjbennet with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-code-for-review-comments

Conversation

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Addresses all review comments from PR #9406 — fixes silent data loss in parameter merging, stale/invalid auth tokens being sent on save, and test correctness issues.

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

Auth token fixes

getAuthorizationToken was returning "******" when no session token existed (template literal with undefined interpolation). Fixed to return '':

// Before
return `******; // → "******" when no session

// After
const token = authData?.accessToken;
return token ? `****** : '';

HttpClient was sending Authorization: undefined as a header string. Fixed all HTTP methods to omit the header entirely when no token:

// Before
headers: { Authorization: isArmId ? this._accessToken : '' }

// After
headers: {
  ...(isArmId && this._accessToken ? { Authorization: this._accessToken } : {}),
}

Access token refresh interval added to LocalDesignerPanel at 30-minute cadence (previous proposal was 5 s — too frequent for Azure AD tokens with ~1 hr TTL). Guards against posting empty/unchanged tokens.

Parameter merge fix (silent data loss)

mergeJsonParameters was discarding file-only properties (metadata, description, etc.) when a parameter existed in both the file and the designer output. Fixed to backfill missing properties:

// Now deep-merges: file-only props are preserved alongside designer-emitted props
for (const prop of Object.keys(fileParam)) {
  if (!(prop in defParam)) {
    defParam[prop] = fileParam[prop];
  }
}

Other fixes

  • Removed dead _clearDirtyState parameter from saveWorkflowFromDesigner and updated DesignerCommandBarProps type accordingly
  • updateDesignerAccessToken action now accepts string | undefined (was string)
  • Added comment in webviewCommunication.tsx explaining why update_access_token dispatches to both WorkflowSlice and DesignerSlice

Impact of Change

  • Users: Workflow saves no longer fail with auth errors when the session token is unavailable at startup; parameter metadata (descriptions, metadata fields) is no longer silently dropped on save
  • Developers: HttpClient.updateAccessToken() now exposed; token refresh wired into panel lifecycle
  • System: No new dependencies; interval cleared on panel dispose to avoid leaks

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

New test files added:

  • getAuthorizationToken.test.ts — covers '' return, tenant passthrough, error propagation, node fallback
  • saveWorkflow.test.ts — covers serialization/error handling with vi.mock('fs') so writeFileSync assertions work
  • designerCommandBarSave.spec.tsx — covers validation-gating logic (save blocked when validation errors present)

Updated tests:

  • httpClient.spec.ts: non-ARM and no-token cases now assert header is absent, not ''

Contributors

Screenshots/Videos

Copilot AI added 3 commits July 17, 2026 20:53
- Remove _clearDirtyState parameter from saveWorkflow (Fix 1)
- Guard against Authorization: undefined in httpClient (Fix 2)
- Fix httpClient.spec.ts to expect header omission (Fix 3)
- Fix updateDesignerAccessToken to accept string | undefined (Fix 4)
- Add 30-min access token refresh interval to localDesignerPanel (Fix 5)
- Deep-merge mergeJsonParameters to preserve file-only properties (Fix 6)
- Fix saveWorkflow.test.ts to mock writeFileSync via vi.mock('fs') (Fix 7)
- Fix getAuthorizationToken to return '' instead of '******' (Fix 8)
- Add comment explaining dual dispatch in webviewCommunication (Fix 9)
- Add test files: getAuthorizationToken.test.ts, saveWorkflow.test.ts, designerCommandBarSave.spec.tsx
- httpClient.spec.ts: Fix test for non-ARM requests to assert Authorization
  header is OMITTED (not empty string), matching conditional-spread implementation
- getAuthorizationToken.test.ts: Fix test description to say 'should return
  empty string' instead of 'should return \"******"' to match assertion
Copilot AI changed the title [WIP] Fix code based on review comments fix(vscode): Fix workflow save token persistence and parameter merge Jul 17, 2026
Copilot AI requested a review from bjbennet July 17, 2026 21:08
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