Skip to content

Conversation

@zanesq
Copy link
Collaborator

@zanesq zanesq commented Oct 29, 2025

Summary

use app.isPackaged instead of checking for node env development

@DOsinga
Copy link
Collaborator

DOsinga commented Nov 4, 2025

I'm going to take this over if you don't mind while you are absent @zanesq

@DOsinga DOsinga marked this pull request as ready for review November 4, 2025 18:08
Copilot AI review requested due to automatic review settings November 4, 2025 18:08
Copy link
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 refactors environment detection across the Electron desktop application by replacing process.env.NODE_ENV === 'development' checks with !app.isPackaged, providing a more reliable way to determine development vs production mode. Additionally, it removes unused development-only testing utilities from the costDatabase module.

Key Changes

  • Standardized environment detection using app.isPackaged instead of NODE_ENV
  • Simplified logger configuration by removing special development-only log paths
  • Removed unused development testing utilities and window object exposure from costDatabase

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ui/desktop/src/utils/pathUtils.ts Simplified development check in path resolution logic
ui/desktop/src/utils/logger.ts Unified log path handling and replaced NODE_ENV checks with app.isPackaged
ui/desktop/src/utils/costDatabase.ts Removed obsolete comment and unused development testing utilities
ui/desktop/src/utils/autoUpdater.ts Replaced NODE_ENV check with app.isPackaged for icon path resolution
ui/desktop/src/main.ts Updated tray icon logic to use app.isPackaged

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 3 to 4
import { app } from 'electron';

Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log file path uses forward slashes which will work on all platforms thanks to Node.js path normalization, but for consistency with the rest of the codebase and clarity, consider using 'path.join(app.getPath('userData'), 'logs', 'main.log')' which is already correct. However, verify that the 'logs' directory will be created automatically. If not, this could cause a runtime error when the app tries to write to a non-existent directory.

Suggested change
import { app } from 'electron';
import { app } from 'electron';
import fs from 'node:fs';
// Ensure the 'logs' directory exists
const logsDir = path.join(app.getPath('userData'), 'logs');
if (!fs.existsSync(logsDir)) {
fs.mkdirSync(logsDir, { recursive: true });
}

Copilot uses AI. Check for mistakes.
@@ -1,4 +1,3 @@
// Import the proper type from ConfigContext
import { getApiUrl } from '../config';
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed comment 'Import the proper type from ConfigContext' which was inaccurate since this file doesn't import anything from ConfigContext.

Copilot uses AI. Check for mistakes.
@alexhancock alexhancock merged commit ff49fa1 into main Nov 4, 2025
20 checks passed
katzdave added a commit that referenced this pull request Nov 4, 2025
…est-and-fix

* 'main' of github.com:block/goose:
  improve linux tray icon support (#5425)
  feat: log rotation (#5561)
  use app.isPackaged instead of checking for node env development (#5465)
  disable RPM build-ID generation to prevent package conflicts (#5563)
  Add Diagnostics Info to Q&A and Bug Report Templates (#5565)
  fix: improve server error messages to include HTTP status code (#5532)
  improvement: add useful error message when attempting to use unauthenticated cursor-agent (#5300)
  fix: unblock acp via databricks (#5562)
  feat: add --output-format json flag to goose run command (#5525)
wpfleger96 added a commit that referenced this pull request Nov 4, 2025
* main: (85 commits)
  improve linux tray icon support (#5425)
  feat: log rotation (#5561)
  use app.isPackaged instead of checking for node env development (#5465)
  disable RPM build-ID generation to prevent package conflicts (#5563)
  Add Diagnostics Info to Q&A and Bug Report Templates (#5565)
  fix: improve server error messages to include HTTP status code (#5532)
  improvement: add useful error message when attempting to use unauthenticated cursor-agent (#5300)
  fix: unblock acp via databricks (#5562)
  feat: add --output-format json flag to goose run command (#5525)
  Sessions required (#5548)
  feat: add grouped extension loading notification (#5529)
  we should run this on main and also test open models at least via ope… (#5556)
  info: print location of sessions.db via goose info (#5557)
  chore: remove yarn usage from documentation (#5555)
  cli: adjust default theme to address #1905 (#5552)
  Manual compaction counting fix + cli cleanup (#5480)
  chore(deps): bump prismjs and react-syntax-highlighter in /ui/desktop (#5549)
  fix: remove qwen3-coder from provider/mcp smoke tests (#5551)
  fix: do not build unsigned desktop app bundles on every PR in ci. add manual option. (#5550)
  fix: update Husky prepare script to v9 format (#5522)
  ...
wpfleger96 added a commit that referenced this pull request Nov 5, 2025
* main: (54 commits)
  add clippy warning for string_slice (#5422)
  improve linux tray icon support (#5425)
  feat: log rotation (#5561)
  use app.isPackaged instead of checking for node env development (#5465)
  disable RPM build-ID generation to prevent package conflicts (#5563)
  Add Diagnostics Info to Q&A and Bug Report Templates (#5565)
  fix: improve server error messages to include HTTP status code (#5532)
  improvement: add useful error message when attempting to use unauthenticated cursor-agent (#5300)
  fix: unblock acp via databricks (#5562)
  feat: add --output-format json flag to goose run command (#5525)
  Sessions required (#5548)
  feat: add grouped extension loading notification (#5529)
  we should run this on main and also test open models at least via ope… (#5556)
  info: print location of sessions.db via goose info (#5557)
  chore: remove yarn usage from documentation (#5555)
  cli: adjust default theme to address #1905 (#5552)
  Manual compaction counting fix + cli cleanup (#5480)
  chore(deps): bump prismjs and react-syntax-highlighter in /ui/desktop (#5549)
  fix: remove qwen3-coder from provider/mcp smoke tests (#5551)
  fix: do not build unsigned desktop app bundles on every PR in ci. add manual option. (#5550)
  ...
michaelneale added a commit that referenced this pull request Nov 6, 2025
* main: (41 commits)
  Add pending extension indicator to extension panel (#5493)
  Add environment subsition for auth blocks (#5439)
  acp: ToolCallLocations and working cancellation (#5588)
  feat(providers): add Mistral AI provider (#5009)
  Listen for ctrl-c during provider request (#5585)
  Also accept null as description, not just missing (#5589)
  Document missing recipe param types (#5584)
  docs: description required for "Add Extension" in cli (#5573)
  fix: Add schema-aware numeric coercion for MCP tool arguments (#5478)
  Add uv for uvx in Justfile (#5581)
  Keep llm logs in place (#5577)
  bump to 1.12.0 (#5580)
  automate more of the release process (#5409)
  add clippy warning for string_slice (#5422)
  improve linux tray icon support (#5425)
  feat: log rotation (#5561)
  use app.isPackaged instead of checking for node env development (#5465)
  disable RPM build-ID generation to prevent package conflicts (#5563)
  Add Diagnostics Info to Q&A and Bug Report Templates (#5565)
  fix: improve server error messages to include HTTP status code (#5532)
  ...
michaelneale added a commit that referenced this pull request Nov 6, 2025
* main: (53 commits)
  acp: ToolCallLocations and working cancellation (#5588)
  feat(providers): add Mistral AI provider (#5009)
  Listen for ctrl-c during provider request (#5585)
  Also accept null as description, not just missing (#5589)
  Document missing recipe param types (#5584)
  docs: description required for "Add Extension" in cli (#5573)
  fix: Add schema-aware numeric coercion for MCP tool arguments (#5478)
  Add uv for uvx in Justfile (#5581)
  Keep llm logs in place (#5577)
  bump to 1.12.0 (#5580)
  automate more of the release process (#5409)
  add clippy warning for string_slice (#5422)
  improve linux tray icon support (#5425)
  feat: log rotation (#5561)
  use app.isPackaged instead of checking for node env development (#5465)
  disable RPM build-ID generation to prevent package conflicts (#5563)
  Add Diagnostics Info to Q&A and Bug Report Templates (#5565)
  fix: improve server error messages to include HTTP status code (#5532)
  improvement: add useful error message when attempting to use unauthenticated cursor-agent (#5300)
  fix: unblock acp via databricks (#5562)
  ...
aharvard added a commit that referenced this pull request Nov 6, 2025
* origin/main: (75 commits)
  fix: customised recipe to yaml string to avoid minininjia parsing error (#5494)
  Add pending extension indicator to extension panel (#5493)
  Add environment subsition for auth blocks (#5439)
  acp: ToolCallLocations and working cancellation (#5588)
  feat(providers): add Mistral AI provider (#5009)
  Listen for ctrl-c during provider request (#5585)
  Also accept null as description, not just missing (#5589)
  Document missing recipe param types (#5584)
  docs: description required for "Add Extension" in cli (#5573)
  fix: Add schema-aware numeric coercion for MCP tool arguments (#5478)
  Add uv for uvx in Justfile (#5581)
  Keep llm logs in place (#5577)
  bump to 1.12.0 (#5580)
  automate more of the release process (#5409)
  add clippy warning for string_slice (#5422)
  improve linux tray icon support (#5425)
  feat: log rotation (#5561)
  use app.isPackaged instead of checking for node env development (#5465)
  disable RPM build-ID generation to prevent package conflicts (#5563)
  Add Diagnostics Info to Q&A and Bug Report Templates (#5565)
  ...
fbalicchia pushed a commit to fbalicchia/goose that referenced this pull request Nov 7, 2025
BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Nov 29, 2025
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.

5 participants