Cleanup a few PR Comments

lib/shared/src/configuration/environment.ts

@@ -21,10 +21,10 @@ import type { UIKind } from 'vscode' // types are ok
 
 export const cenv = defineEnvBuilder({
     /**
-     * A combination of HTTP_PROXY, HTTPS_PROXY and NO_PROXY environment
-     * variables for easier consumption.
+     * A proxy string that falls back to Node's HTTP_PROXY and HTTPS_PROXY if
+     * not explicitly configured (or disabled with 'false').
      */
-    CODY_NODE_DEFAULT_PROXY: codyProxy,
+    CODY_NODE_DEFAULT_PROXY: proxyStringWithNodeFallback,
 
     /**
      * Enables unstable internal testing configuration to be read from settings.json
@@ -161,13 +161,14 @@ function uiKind(uiKind: string | undefined): UIKind | undefined {
     }
 }
 
-function codyProxy(envValue: string | undefined): string | undefined {
-    switch (bool(envValue)) {
-        case false:
-            // explicitly disabled
-            return undefined
-        default:
-            break
+/**
+ * If explicitly set to false no proxy value is returned. Or returns the string
+ * value set otherwise. If no explicit value is set value defaults to the
+ * default HTTPS_PROXY or HTTP_PROXY values (in that order).
+ */
+function proxyStringWithNodeFallback(envValue: string | undefined): string | undefined {
+    if (bool(envValue) === false) {
+        return undefined
     }
 
     const forcedProxy = str(envValue)

lib/shared/src/fetch.patch.ts

@@ -2,14 +2,15 @@
  * In node environments, it might be necessary to set up a custom agent to
  * control the network requests being made.
  *
- * To do this, we have a mutable agent variable that can be set externally. For
- * this to work we need to ensure no other dependencies have loaded yet. That's
- * why this module is seperate from the fetch.ts file which simply re-exports
- * this variable. This way the patching code can simply import this specific
- * file and gain early access to the agentRef
+ * To do this, we expose the globalAgentRef variable that can be mutated
+ * externally.
  *
  * 🚨 Do not import this file unless you're specifically patching the variable.
- * For read-only access simply import the fetch.ts file
+ * For read-only access simply import the fetch.ts file. This variable was only
+ * isolated into this separate .patch file so that patching code can import this
+ * without bringing in transitive dependencies.
+ *
+ * An example of such a patch where this is critical is in `vscode/src/net/net.patch.ts`
  */
 
 import type EventEmitter from 'node:events'
@@ -19,11 +20,11 @@ import type { Agent } from 'agent-base'
 
 let _globalAgent: Agent | undefined
 let _eventEmitter: EventEmitter<NetEventMap> | undefined
-let _blockEarlyAccess = false
 export const globalAgentRef = {
     get agent() {
-        if (_blockEarlyAccess && !_globalAgent) {
-            throw new Error('Agent was used before it was initialized')
+        // We can't import logging here so console will have to suffice
+        if (!globalAgentRef.isSet) {
+            console.error('Agent was used before it was initialized')
         }
         return _globalAgent
     },
@@ -41,10 +42,6 @@ export const globalAgentRef = {
         _eventEmitter = v
     },
 
-    set blockEarlyAccess(v: boolean) {
-        _blockEarlyAccess = v
-    },
-
     get isSet() {
         return _globalAgent !== undefined
     },

vscode/src/net/net.patch.ts

@@ -1,4 +1,4 @@
-// 🚨 This patch file mast have NO additional dependencies as import order
+// 🚨 This patch file must have NO additional dependencies as import order
 // changes might cause modules to load before the networking is patched.
 import EventEmitter from 'node:events'
 import type * as http from 'node:http'
@@ -11,8 +11,6 @@ import type * as internalVSCodeAgent from './vscode-network-proxy'
 export const bypassVSCodeSymbol = Symbol('bypassVSCodeSymbol')
 let patched = false
 patchNetworkStack()
-// This needs to happen after so that we don't break import order
-globalAgentRef.blockEarlyAccess = true
 
 function patchNetworkStack(): void {
     if (patched) {