Merge pull request #93 from microsoft/alzollin/caller

azchohfi · web-flow · commit d0b693210424 · 2025-11-04T12:33:49.000-08:00
Properly handling caller and other env variables.
diff --git a/src/winapp-CLI/WinApp.Cli.Tests/WinappDirectoryServiceTests.cs b/src/winapp-CLI/WinApp.Cli.Tests/WinappDirectoryServiceTests.cs
@@ -76,15 +76,15 @@ public void SetCacheDirectoryForTesting_CanBeChangedMultipleTimes()
     public void GetGlobalWinappDirectory_WithEnvironmentVariable_ReturnsEnvironmentVariablePath()
     {
         // Store original environment variable value to restore later
-        var originalValue = Environment.GetEnvironmentVariable("WINAPP_CACHE_DIRECTORY");
+        var originalValue = Environment.GetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY");
         
         // Arrange - Create a test directory for environment variable
         var envTestDirectory = _tempDirectory.CreateSubdirectory("env-test-winapp");
 
         try
         {
             // Set environment variable
-            Environment.SetEnvironmentVariable("WINAPP_CACHE_DIRECTORY", envTestDirectory.FullName);
+            Environment.SetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY", envTestDirectory.FullName);
 
             // Act - Create fresh instance to test environment variable behavior
             var directoryService = GetRequiredService<IWinappDirectoryService>();
@@ -96,7 +96,7 @@ public void GetGlobalWinappDirectory_WithEnvironmentVariable_ReturnsEnvironmentV
         finally
         {
             // Cleanup - Restore original environment variable value
-            Environment.SetEnvironmentVariable("WINAPP_CACHE_DIRECTORY", originalValue);
+            Environment.SetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY", originalValue);
         }
     }
 
@@ -105,7 +105,7 @@ public void GetGlobalWinappDirectory_WithEnvironmentVariable_ReturnsEnvironmentV
     public void SetCacheDirectoryForTesting_TakesPrecedenceOverEnvironmentVariable()
     {
         // Store original environment variable value to restore later
-        var originalValue = Environment.GetEnvironmentVariable("WINAPP_CACHE_DIRECTORY");
+        var originalValue = Environment.GetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY");
         
         // Arrange - Create test directories
         var envTestDirectory = _tempDirectory.CreateSubdirectory("env-winapp");
@@ -114,7 +114,7 @@ public void SetCacheDirectoryForTesting_TakesPrecedenceOverEnvironmentVariable()
         try
         {
             // Set environment variable
-            Environment.SetEnvironmentVariable("WINAPP_CACHE_DIRECTORY", envTestDirectory.FullName);
+            Environment.SetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY", envTestDirectory.FullName);
             
             // Act - Create instance and set override (should take precedence)
             var directoryService = GetRequiredService<IWinappDirectoryService>();
@@ -127,7 +127,7 @@ public void SetCacheDirectoryForTesting_TakesPrecedenceOverEnvironmentVariable()
         finally
         {
             // Cleanup - Restore original environment variable value
-            Environment.SetEnvironmentVariable("WINAPP_CACHE_DIRECTORY", originalValue);
+            Environment.SetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY", originalValue);
         }
     }
 
diff --git a/src/winapp-CLI/WinApp.Cli/Services/WinappDirectoryService.cs b/src/winapp-CLI/WinApp.Cli/Services/WinappDirectoryService.cs
@@ -28,7 +28,7 @@ public DirectoryInfo GetGlobalWinappDirectory()
         }
 
         // Allow override via environment variable (useful for CI/CD)
-        var cacheDirectory = Environment.GetEnvironmentVariable("WINAPP_CACHE_DIRECTORY");
+        var cacheDirectory = Environment.GetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY");
         if (!string.IsNullOrEmpty(cacheDirectory))
         {
             return new DirectoryInfo(cacheDirectory);
diff --git a/src/winapp-CLI/WinApp.Cli/Telemetry/Events/EventBase.cs b/src/winapp-CLI/WinApp.Cli/Telemetry/Events/EventBase.cs
@@ -33,6 +33,8 @@ public abstract PartA_PrivTags PartA_PrivTags
 
     public bool CI { get; } = CIEnvironmentDetectorForTelemetry.IsCIEnvironment();
 
+    public string? Caller { get; set; } = Environment.GetEnvironmentVariable("WINAPP_CLI_CALLER");
+
     /// <summary>
     /// Replaces all the strings in this event that may contain PII using the provided function.
     /// </summary>
diff --git a/src/winapp-CLI/WinApp.Cli/Telemetry/Telemetry.cs b/src/winapp-CLI/WinApp.Cli/Telemetry/Telemetry.cs
@@ -103,7 +103,7 @@ internal Telemetry()
     /// Gets a value indicating whether telemetry is on
     /// For future use if we add a registry key or some other setting to check if telemetry is turned on.
     /// </summary>
-    public bool IsTelemetryOn { get; } = true;
+    public bool IsTelemetryOn { get; } = Environment.GetEnvironmentVariable("WINAPP_CLI_TELEMETRY_OPTOUT") != "1";
 
     /// <summary>
     /// Gets or sets a value indicating whether diagnostic telemetry is on.
@@ -213,6 +213,7 @@ public void LogCritical(string eventName, bool isError = false, Guid? relatedAct
         where T : EventBase
     {
         data.ReplaceSensitiveStrings(this.ReplaceSensitiveStrings);
+        data.Caller = data.Caller != null ? this.ReplaceSensitiveStrings(data.Caller) : null;
         this.LogInternal(eventName, level, data, relatedActivityId, isError: false);
     }
 
@@ -228,6 +229,7 @@ public void LogCritical(string eventName, bool isError = false, Guid? relatedAct
         where T : EventBase
     {
         data.ReplaceSensitiveStrings(this.ReplaceSensitiveStrings);
+        data.Caller = data.Caller != null ? this.ReplaceSensitiveStrings(data.Caller) : null;
         this.LogInternal(eventName, level, data, relatedActivityId, isError: true);
     }
 
diff --git a/src/winapp-GUI/winapp-GUI/Properties/launchSettings.json b/src/winapp-GUI/winapp-GUI/Properties/launchSettings.json
@@ -0,0 +1,10 @@
+{
+  "profiles": {
+    "winapp-GUI (Package)": {
+      "commandName": "MsixPackage"
+    },
+    "winapp-GUI (Unpackaged)": {
+      "commandName": "Project"
+    }
+  }
+}
diff --git a/src/winapp-GUI/winapp-GUI/Services/CliService.cs b/src/winapp-GUI/winapp-GUI/Services/CliService.cs
@@ -46,7 +46,11 @@ public async Task<int> RunCliStandardizedAsync(string title, string arguments, s
             Arguments = arguments,
             WorkingDirectory = workingDir,
             CreateNoWindow = true,
-            WindowStyle = ProcessWindowStyle.Hidden
+            WindowStyle = ProcessWindowStyle.Hidden,
+            EnvironmentVariables =
+            {
+                ["WINAPP_CLI_CALLER"] = "winapp-GUI"
+            }
         };
         if (runAsAdmin)
         {
diff --git a/src/winapp-npm/cli.js b/src/winapp-npm/cli.js
@@ -3,7 +3,7 @@
 const { generateAddonFiles } = require('./addon-utils');
 const { generateCsAddonFiles } = require('./cs-addon-utils');
 const { addElectronDebugIdentity } = require('./msix-utils');
-const { getWinappCliPath, callWinappCli } = require('./winapp-cli-utils');
+const { getWinappCliPath, callWinappCli, WINAPP_CLI_CALLER_VALUE } = require('./winapp-cli-utils');
 const { spawn } = require('child_process');
 const path = require('path');
 const fs = require('fs');
@@ -81,7 +81,11 @@ async function showCombinedHelp() {
     await new Promise((resolve, reject) => {
       const child = spawn(winappCliPath, ['--help'], {
         stdio: 'inherit',
-        shell: false
+        shell: false,
+        env: {
+          ...process.env,
+          WINAPP_CLI_CALLER: WINAPP_CLI_CALLER_VALUE
+        }
       });
       
       child.on('close', (code) => {
@@ -133,7 +137,11 @@ async function showVersion() {
     await new Promise((resolve, reject) => {
       const child = spawn(winappCliPath, ['--version'], {
         stdio: 'inherit',
-        shell: false
+        shell: false,
+        env: {
+          ...process.env,
+          WINAPP_CLI_CALLER: WINAPP_CLI_CALLER_VALUE
+        }
       });
       
       child.on('close', (code) => {
diff --git a/src/winapp-npm/winapp-cli-utils.js b/src/winapp-npm/winapp-cli-utils.js
@@ -3,6 +3,8 @@ const path = require('path');
 const os = require('os');
 const { spawn } = require('child_process');
 
+const WINAPP_CLI_CALLER_VALUE = 'nodejs-package';
+
 /**
  * Helper function to get the path to the winapp-cli executable
  */
@@ -38,7 +40,11 @@ async function callWinappCli(args, options = {}) {
     const child = spawn(winappCliPath, args, {
       stdio: verbose ? 'inherit' : 'pipe',
       cwd: process.cwd(),
-      shell: false
+      shell: false,
+      env: {
+        ...process.env,
+        WINAPP_CLI_CALLER: WINAPP_CLI_CALLER_VALUE
+      }
     });
     
     // Only capture stderr when not using inherit mode (needed for error messages)
@@ -78,5 +84,6 @@ async function callWinappCli(args, options = {}) {
 
 module.exports = {
   getWinappCliPath: getWinappCliPath,
-  callWinappCli: callWinappCli
+  callWinappCli: callWinappCli,
+  WINAPP_CLI_CALLER_VALUE: WINAPP_CLI_CALLER_VALUE
 };

Original file line number	Diff line number	Diff line change
`@@ -76,15 +76,15 @@ public void SetCacheDirectoryForTesting_CanBeChangedMultipleTimes()`
`76`	`76`	`public void GetGlobalWinappDirectory_WithEnvironmentVariable_ReturnsEnvironmentVariablePath()`
`77`	`77`	`{`
`78`	`78`	`// Store original environment variable value to restore later`
`79`		`- var originalValue = Environment.GetEnvironmentVariable("WINAPP_CACHE_DIRECTORY");`
	`79`	`+ var originalValue = Environment.GetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY");`
`80`	`80`
`81`	`81`	`// Arrange - Create a test directory for environment variable`
`82`	`82`	`var envTestDirectory = _tempDirectory.CreateSubdirectory("env-test-winapp");`
`83`	`83`
`84`	`84`	`try`
`85`	`85`	`{`
`86`	`86`	`// Set environment variable`
`87`		`- Environment.SetEnvironmentVariable("WINAPP_CACHE_DIRECTORY", envTestDirectory.FullName);`
	`87`	`+ Environment.SetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY", envTestDirectory.FullName);`
`88`	`88`
`89`	`89`	`// Act - Create fresh instance to test environment variable behavior`
`90`	`90`	`var directoryService = GetRequiredService<IWinappDirectoryService>();`
`@@ -96,7 +96,7 @@ public void GetGlobalWinappDirectory_WithEnvironmentVariable_ReturnsEnvironmentV`
`96`	`96`	`finally`
`97`	`97`	`{`
`98`	`98`	`// Cleanup - Restore original environment variable value`
`99`		`- Environment.SetEnvironmentVariable("WINAPP_CACHE_DIRECTORY", originalValue);`
	`99`	`+ Environment.SetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY", originalValue);`
`100`	`100`	`}`
`101`	`101`	`}`
`102`	`102`
`@@ -105,7 +105,7 @@ public void GetGlobalWinappDirectory_WithEnvironmentVariable_ReturnsEnvironmentV`
`105`	`105`	`public void SetCacheDirectoryForTesting_TakesPrecedenceOverEnvironmentVariable()`
`106`	`106`	`{`
`107`	`107`	`// Store original environment variable value to restore later`
`108`		`- var originalValue = Environment.GetEnvironmentVariable("WINAPP_CACHE_DIRECTORY");`
	`108`	`+ var originalValue = Environment.GetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY");`
`109`	`109`
`110`	`110`	`// Arrange - Create test directories`
`111`	`111`	`var envTestDirectory = _tempDirectory.CreateSubdirectory("env-winapp");`
`@@ -114,7 +114,7 @@ public void SetCacheDirectoryForTesting_TakesPrecedenceOverEnvironmentVariable()`
`114`	`114`	`try`
`115`	`115`	`{`
`116`	`116`	`// Set environment variable`
`117`		`- Environment.SetEnvironmentVariable("WINAPP_CACHE_DIRECTORY", envTestDirectory.FullName);`
	`117`	`+ Environment.SetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY", envTestDirectory.FullName);`
`118`	`118`
`119`	`119`	`// Act - Create instance and set override (should take precedence)`
`120`	`120`	`var directoryService = GetRequiredService<IWinappDirectoryService>();`
`@@ -127,7 +127,7 @@ public void SetCacheDirectoryForTesting_TakesPrecedenceOverEnvironmentVariable()`
`127`	`127`	`finally`
`128`	`128`	`{`
`129`	`129`	`// Cleanup - Restore original environment variable value`
`130`		`- Environment.SetEnvironmentVariable("WINAPP_CACHE_DIRECTORY", originalValue);`
	`130`	`+ Environment.SetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY", originalValue);`
`131`	`131`	`}`
`132`	`132`	`}`
`133`	`133`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ public DirectoryInfo GetGlobalWinappDirectory()`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`// Allow override via environment variable (useful for CI/CD)`
`31`		`- var cacheDirectory = Environment.GetEnvironmentVariable("WINAPP_CACHE_DIRECTORY");`
	`31`	`+ var cacheDirectory = Environment.GetEnvironmentVariable("WINAPP_CLI_CACHE_DIRECTORY");`
`32`	`32`	`if (!string.IsNullOrEmpty(cacheDirectory))`
`33`	`33`	`{`
`34`	`34`	`return new DirectoryInfo(cacheDirectory);`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ internal Telemetry()`
`103`	`103`	`/// Gets a value indicating whether telemetry is on`
`104`	`104`	`/// For future use if we add a registry key or some other setting to check if telemetry is turned on.`
`105`	`105`	`/// </summary>`
`106`		`- public bool IsTelemetryOn { get; } = true;`
	`106`	`+ public bool IsTelemetryOn { get; } = Environment.GetEnvironmentVariable("WINAPP_CLI_TELEMETRY_OPTOUT") != "1";`
`107`	`107`
`108`	`108`	`/// <summary>`
`109`	`109`	`/// Gets or sets a value indicating whether diagnostic telemetry is on.`
`@@ -213,6 +213,7 @@ public void LogCritical(string eventName, bool isError = false, Guid? relatedAct`
`213`	`213`	`where T : EventBase`
`214`	`214`	`{`
`215`	`215`	`data.ReplaceSensitiveStrings(this.ReplaceSensitiveStrings);`
	`216`	`+ data.Caller = data.Caller != null ? this.ReplaceSensitiveStrings(data.Caller) : null;`
`216`	`217`	`this.LogInternal(eventName, level, data, relatedActivityId, isError: false);`
`217`	`218`	`}`
`218`	`219`
`@@ -228,6 +229,7 @@ public void LogCritical(string eventName, bool isError = false, Guid? relatedAct`
`228`	`229`	`where T : EventBase`
`229`	`230`	`{`
`230`	`231`	`data.ReplaceSensitiveStrings(this.ReplaceSensitiveStrings);`
	`232`	`+ data.Caller = data.Caller != null ? this.ReplaceSensitiveStrings(data.Caller) : null;`
`231`	`233`	`this.LogInternal(eventName, level, data, relatedActivityId, isError: true);`
`232`	`234`	`}`
`233`	`235`