Skip to content

fix: restore protected modifier on relayCreationTimeoutMs in test helper#316049

Merged
connor4312 merged 2 commits into
microsoft:mainfrom
CodeEditorLand:fix-protected-override-1
May 13, 2026
Merged

fix: restore protected modifier on relayCreationTimeoutMs in test helper#316049
connor4312 merged 2 commits into
microsoft:mainfrom
CodeEditorLand:fix-protected-override-1

Conversation

@NikolaRHristov
Copy link
Copy Markdown
Contributor

Same as #310195 as we're running into:

[19:19:00] [mangler] Done collecting. Classes: 10966. Exported symbols: 13574
[19:19:01] [mangler] WARN: 'relayCreationTimeoutMs' from src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts:520 became PUBLIC because of: src/vs/platform/agentHost/test/node/sshRemoteAgentHostService.test.ts:177
[19:19:01] [mangler] ERROR: Protected fields have been made PUBLIC. This hurts minification and is therefore not allowed. Review the WARN messages further above
[19:19:01] Starting compilation...
[19:19:01] 'compile-build-with-mangling' errored after 14 s
[19:19:01] Error: Protected fields have been made PUBLIC. This hurts minification and is therefore not allowed. Review the WARN messages further above
    at Mangler.computeNewFileContents (file://build/lib/mangle/index.ts:568:10)
    at task (file://build/lib/compilation.ts:144:47)
    at file://build/lib/task.ts:62:22
    at new Promise (<anonymous>)
    at _doExecute (file://build/lib/task.ts:50:9)
    at _execute (file://build/lib/task.ts:40:8)
    at result (file://build/lib/task.ts:85:10)
    at process.processTicksAndRejections (node:internal/process/task_queues:103:5)
    at async _execute (file://build/lib/task.ts:40:2)
    at async result (file://build/lib/task.ts:85:4)

after npm install -> npm run compile-build

The `relayCreationTimeoutMs` override in `TestableSSHRemoteAgentHostMainService`
was missing its `protected` modifier, causing the mangler to promote it to
`public` and fail the `compile-build-with-mangling` step with:

  ERROR: Protected fields have been made PUBLIC. This hurts minification
  and is therefore not allowed.

Re-add `protected` to the override declaration to keep the field's
visibility consistent with the base class and satisfy the mangler.
Copilot AI review requested due to automatic review settings May 12, 2026 16:31
@NikolaRHristov
Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree company="PlayForm"

Copy link
Copy Markdown
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 aims to fix compile-build-with-mangling failures by preventing the mangler from promoting SSHRemoteAgentHostMainService.relayCreationTimeoutMs to public due to a public override in the test subclass.

Changes:

  • Change TestableSSHRemoteAgentHostMainService.relayCreationTimeoutMs from override (effectively public) to protected override to match the base class and avoid mangler promotion errors.

Comment on lines 176 to +177
/** Public override so tests can shorten the relay creation timeout. */
override relayCreationTimeoutMs: number = 30_000;
protected override relayCreationTimeoutMs: number = 30_000;
@connor4312 connor4312 enabled auto-merge (squash) May 12, 2026 17:28
connor4312
connor4312 previously approved these changes May 12, 2026
…ngler

The field `relayCreationTimeoutMs` is `protected` in the base class. The
test subclass `TestableSSHRemoteAgentHostMainService` had overridden it
without an explicit access modifier, making it implicitly `public`. The
mangler correctly rejected this as it hurts minification.

Adding `protected` back satisfied the mangler but caused a TS2445 error
since the test was assigning to the field directly on an instance, which
is illegal for `protected` members outside the class hierarchy.

Fix by adding a narrow public setter `setRelayCreationTimeoutForTest` in
the test subclass. The field stays `protected`; the setter provides a
legitimate public entry point for tests.

Signed-off-by: Nikola Hristov <Nikola@PlayForm.Cloud>
auto-merge was automatically disabled May 12, 2026 17:32

Head branch was pushed to by a user without write access

@NikolaRHristov
Copy link
Copy Markdown
Contributor Author

A follow up, as we now encountered:

error TS2445: Property 'relayCreationTimeoutMs' is protected and only accessible within class 'TestableSSHRemoteAgentHostMainService' and its subclasses.

1020   service.relayCreationTimeoutMs = 50;
               ~~~~~~~~~~~~~~~~~~~~~~

so an additional commit was made specifically for the test - c5061c3

@connor4312 connor4312 merged commit bb200f4 into microsoft:main May 13, 2026
28 of 33 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.121.0 milestone May 13, 2026
NikolaRHristov pushed a commit to CodeEditorLand/Editor that referenced this pull request May 13, 2026
…-override-1

fix: restore protected modifier on relayCreationTimeoutMs in test helper
@NikolaRHristov NikolaRHristov deleted the fix-protected-override-1 branch May 19, 2026 23:37
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.

4 participants