Skip to content

feat(desktop): let distribution packages point at their own resources - #14795

Open
limwa wants to merge 1 commit into
pingdotgg:mainfrom
limwa:feat/desktop-resources-path
Open

limwa wants to merge 1 commit into
pingdotgg:mainfrom
limwa:feat/desktop-resources-path

Conversation

@limwa

@limwa limwa commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Problem

Linux distribution packages (Nix, Arch, …) run the app's app.asar on the distribution's shared Electron. process.resourcesPath is then that Electron's own resources directory, so the desktop app can't find any of the resources the build places next to app.asar. On Linux that breaks the GNOME extension setup, the KDE and Hyprland capture helpers, the browser-secret helper and the resource monitor. For example, SnapShots setup fails with:

ENOENT: no such file or directory, open '/nix/store/…-electron-unwrapped-44.3.0/libexec/electron/resources/gnome-extension/metadata.json'

Change

T3CODE_DESKTOP_RESOURCES_PATH names the app's resources directory. DesktopEnvironment resolves it once, and every lookup already goes through environment.resourcesPath: the helpers above, app-update.yml, package-type, and the Windows server.asar root. When it's unset, process.resourcesPath is used as before, so normal builds don't change.

I chose an explicit setting over inferring the directory from the loaded .asar, because packages may run a plain app directory or keep the resources in a separate location. A short note in docs/operations/development.md tells packagers about it.

Scope and approval

There's no prior issue or discussion. This is focused configuration of an existing capability: the desktop app already reads these resources from one directory, and the option only sets where that directory is. With the variable unset, nothing changes. It pairs with #8668, which gives distribution packages a stable t3code:// launcher.

Verification

  • New DesktopEnvironment test: a packaged Linux app on a shared Electron reads resourcesPath, app-update.yml and the icon candidates from the configured directory. The existing environment tests still pass (8 tests).
  • Lint and formatting pass on the changed files, and the desktop typecheck reports no errors in desktop sources.
  • Manually: before this change, an Electron 44 nixpkgs build hit the ENOENT above when setting up the GNOME extension. I haven't rerun that manual setup with the variable set.

Implemented with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

Distribution packages run the app on a shared Electron, whose
process.resourcesPath is that Electron's own resources directory. The
desktop app then looks there for the GNOME extension, the KDE and Hyprland
capture helpers, the browser-secret helper and the resource monitor, and
finds none of them. T3CODE_DESKTOP_RESOURCES_PATH lets the package name its
own resources directory; unset, nothing changes.

Assisted-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 2, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 2, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4c2803c

Macroscope's review found this PR approvable — Adds an optional resource-directory override for distribution packages while preserving the existing Electron resource path when unset. The centralized path change is covered by tests and affects only existing resource lookups, with no product-default or static-analysis changes.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 2, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 2, 2026 11:42

Dismissing prior approval to re-evaluate 4c2803c

@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ab088ac1-9829-4b68-b1e2-6543e80cc070

📥 Commits

Reviewing files that changed from the base of the PR and between 54084ae and 4c2803c.

📒 Files selected for processing (4)
  • apps/desktop/src/app/DesktopConfig.ts
  • apps/desktop/src/app/DesktopEnvironment.test.ts
  • apps/desktop/src/app/DesktopEnvironment.ts
  • docs/operations/development.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Desktop configuration now reads T3CODE_DESKTOP_RESOURCES_PATH. The desktop environment uses the resolved override for resource paths when it is set. A test and operations documentation cover shared-Electron package launches.

Changes

Desktop resources path override

Layer / File(s) Summary
Configure and resolve resources path
apps/desktop/src/app/DesktopConfig.ts, apps/desktop/src/app/DesktopEnvironment.ts, apps/desktop/src/app/DesktopEnvironment.test.ts, docs/operations/development.md
Configuration reads the trimmed environment setting. The desktop environment resolves and uses the override, and a test checks its use in a packaged Linux environment. Operations documentation describes the setting for shared-Electron package launches.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 4c280

Packagers can supply the app’s resources directory as documented; no issue identified here needs resolution before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4c280

The override preserves default behavior and does not demonstrate a new remote attack path or privilege elevation. However, changing the resource directory can leave Windows WSL fallback execution using previously cached backend code. Safe deployment also depends on protecting the launch configuration and selected directory.

Retained concerns

  • Medium · reliability · inferred: Changing the configured resources directory without changing appVersion can leave packaged Windows WSL fallback execution using the previous directory's extracted backend. The cache location and completeness marker contain only the version, so a completed cache bypasses the newly selected server.asar. This cache policy predates the PR, but independently selectable resource roots introduce a new source-switching path that the policy does not distinguish. It can undermine resource replacement or rollback intended to remove an unwanted backend.
Security review details

Security Blast Radius

  • inferred — Compromise of a selected executable resource can affect the launching user's desktop execution context, browser-secret helper activity and, on Windows, the backend and selected WSL runtime. No cross-tenant, cloud-IAM or new OS privilege transition was established by the inspected paths.

Security Findings and Attack Paths

  • inferred — An attacker-controlled launch setting or writable selected directory could redirect helper or backend execution. That is a conditional trust requirement, not a verified exploit: the evidence does not establish a lower-trust actor able to control either input. The concrete remaining concern is source identity during WSL fallback reuse.

Trust Boundaries and Controls

  • observed — The packaged Linux browser-secret helper uses a fixed relative filename under the selected root rather than PATH lookup. Missing helpers produce an unavailable-key error. These controls constrain lookup, but do not authenticate the directory or executable; deployment ownership protections remain unverified.

Resilience and Maintainability Implications

  • inferred — Successful content-identified WSL staging limits the stale-source concern by removing legacy extraction state. Archive absence or staging failure leaves the mounted fallback relevant, so resource replacement should not assume that changing the root also replaces every executable cache.

Hardening Proposals

  • proposed — Bind WSL fallback cache validity to the selected source identity, or explicitly invalidate it when resource selection changes, so same-version source replacement and rollback cannot silently reuse the prior backend.
  • proposed — Document the setting as a trusted executable-resource binding: package launchers should select a protected absolute directory and prevent lower-trust modification of its contents or launch configuration. This is a deployment hardening proposal, not an observed ownership failure.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: allowing distribution packages to select their own desktop resources directory.
Description check ✅ Passed The description covers the problem, implementation, scope rationale, approval context, verification results, and an explicit limitation about the manual check that was not rerun.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants