Skip to content

refactor: freeze DEFAULT_CONFIG to prevent shared-state mutation - #584

Merged
askpt merged 3 commits into
mainfrom
repo-assist/improve-freeze-default-config-20260830-782b94cdfba40099
Aug 30, 2026
Merged

refactor: freeze DEFAULT_CONFIG to prevent shared-state mutation#584
askpt merged 3 commits into
mainfrom
repo-assist/improve-freeze-default-config-20260830-782b94cdfba40099

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

🤖 This pull request was created by Repo Assist, an automated AI assistant.

Summary

Freezes DEFAULT_CONFIG (and its excludePatterns array) in src/configuration.ts to guard against a latent shared-mutable-state bug.

Why

VS Code's WorkspaceConfiguration.get(key, defaultValue) returns the exact same reference to defaultValue whenever the user hasn't overridden that setting — it does not clone. That means every call to ConfigurationManager.getConfiguration() for a workspace without a custom excludePatterns setting returns the same array object as DEFAULT_CONFIG.excludePatterns.

If any current or future code accidentally mutated that returned array (e.g. .push(), .sort()), it would silently corrupt the shared default for the rest of the extension's session across all workspace folders — a subtle bug that would be very hard to trace back to its cause.

Fix

Wrap DEFAULT_CONFIG and its excludePatterns array in Object.freeze(). This is a zero-behavior-change, defensive hardening: no current code path mutates the array, but freezing costs nothing and converts a future accidental mutation into a loud failure (in strict mode) or a silent no-op, instead of quiet corruption.

Trade-offs

None — purely additive safety, no API or behavior change.

Test Status

  • npm run compile
  • npm run lint
  • npm run test:unit ✅ 241 passing, coverage 98.77/95.16/99.1/98.77 (thresholds 95/88/97/95, unchanged)
  • npm test (full, requires VS Code download) not run — blocked by sandbox network restrictions (expected/known limitation, unrelated to this change)

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Generated by 🌈 Repo Assist, see workflow run. Learn more.
Comment /repo-assist to run again

Add this agentic workflow to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@42c2ab5b4e4c9273534c39259b2e0df7f20f07e9

VS Code's WorkspaceConfiguration.get(key, defaultValue) returns the exact
same defaultValue reference when a setting isn't overridden by the user.
This means every call to getConfiguration() for a workspace without a
custom excludePatterns setting returns the same array object as
DEFAULT_CONFIG.excludePatterns. Any accidental mutation of that array by
current or future code would silently corrupt the shared defaults for the
rest of the session.

Freeze DEFAULT_CONFIG and its excludePatterns array to make such mutations
throw (strict mode) or no-op instead of corrupting shared state.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@askpt askpt changed the title [repo-assist] refactor: freeze DEFAULT_CONFIG to prevent shared-state mutation refactor: freeze DEFAULT_CONFIG to prevent shared-state mutation Aug 30, 2026
@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.11%. Comparing base (7f062d3) to head (897d8d2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #584   +/-   ##
=======================================
  Coverage   98.11%   98.11%           
=======================================
  Files          12       12           
  Lines        3931     3931           
  Branches      451      451           
=======================================
  Hits         3857     3857           
  Misses         74       74           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@askpt
askpt marked this pull request as ready for review August 30, 2026 12:14
@askpt
askpt self-requested a review as a code owner August 30, 2026 12:14
Copilot AI balanced review requested due to automatic review settings August 30, 2026 12:14

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

The mutable type contract conflicts with frozen runtime values, and the new behavior lacks targeted tests.

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

Pull request overview

Freezes shared default configuration values to prevent accidental mutation across workspace scopes.

Changes:

  • Freezes DEFAULT_CONFIG and its nested exclusion patterns.
  • Documents the shared-reference rationale.
File summaries
File Description
src/configuration.ts Adds defensive freezing for defaults.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Balanced

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

Comment thread src/configuration.ts
Comment thread src/configuration.ts Outdated
@askpt

askpt commented Aug 30, 2026

Copy link
Copy Markdown
Owner

@copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review thread.

Copilot AI and others added 2 commits August 30, 2026 12:30
Co-authored-by: askpt <2493377+askpt@users.noreply.github.com>
…freeze behavior

Co-authored-by: askpt <2493377+askpt@users.noreply.github.com>

Copilot AI commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

@copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply t...

Addressed both review points in commits refactor: use readonly types… and refactor: freeze returned excludePatterns…:

  1. Type contractexcludePatterns in CodeMetricsConfig is now readonly string[], the as string[] cast is removed, and getCompiledPatterns/isExcluded in codeLensProvider.ts accept readonly string[].

  2. Freeze coveragegetConfiguration() now wraps the returned array in Object.freeze(), so user-supplied arrays are frozen too (consistent with DEFAULT_CONFIG).

  3. Tests — Added three targeted tests:

    • DEFAULT_CONFIG and DEFAULT_CONFIG.excludePatterns are frozen (Object.isFrozen).
    • getConfiguration().excludePatterns is frozen.
    • Attempted mutation of DEFAULT_CONFIG.excludePatterns leaves the length unchanged.

@askpt
askpt enabled auto-merge (squash) August 30, 2026 16:57
@askpt
askpt merged commit 3d30dc8 into main Aug 30, 2026
10 checks passed
@askpt
askpt deleted the repo-assist/improve-freeze-default-config-20260830-782b94cdfba40099 branch August 30, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants