chore: sync upstream fixes (SSH, sidebar, menu copy, CI, drag)#22
Merged
aaditagrawal merged 7 commits intomainfrom Mar 18, 2026
Merged
chore: sync upstream fixes (SSH, sidebar, menu copy, CI, drag)#22aaditagrawal merged 7 commits intomainfrom
aaditagrawal merged 7 commits intomainfrom
Conversation
…otgg#972) Co-authored-by: Julius Marminge <julius0216@outlook.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors desktop shell environment syncing and shared shell readers, adds macOS-targeted PATH/SSH_AUTH_SOCK hydration and tests, updates main to use the new utility, enhances web sidebar clipboard and cursor behavior, bumps MSW version, and extends the release workflow to mint and use a GitHub App token for commit identity. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
github-actions
bot
added
size:L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/release.yml (1)
282-289: Scope the minted GitHub App token to the current repository and required permissions (least privilege).The token scope currently depends on app installation defaults. Explicitly restrict it to the current repository and set
contents: writepermission so future app-scope changes don't silently widen workflow privileges.🔐 Suggested hardening diff
- id: app_token name: Mint release app token uses: actions/create-github-app-token@v2 with: app-id: ${{ vars.RELEASE_APP_ID }} private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} owner: ${{ github.repository_owner }} + repositories: ${{ github.event.repository.name }} + permission-contents: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 282 - 289, The GitHub App token step (id: app_token, uses: actions/create-github-app-token@v2) is unscoped; update the step to explicitly restrict the token to the current repository and least-privilege permissions by adding the repository input set to the current repo (use the github.repository expression) and a permissions block that at minimum sets contents: write (and any other specific minimal permissions required), so the token no longer inherits broad installation defaults.apps/desktop/src/syncShellEnvironment.test.ts (1)
5-85: Add coverage for the swallowed-error path.The suite never asserts the behavior that keeps desktop startup resilient: if the login-shell probe throws,
syncShellEnvironmentshould leave the inherited environment untouched. That branch matters now thatapps/desktop/src/main.tscalls this during module initialization.🧪 Proposed test
+ it("keeps the inherited environment when login shell probing fails", () => { + const env: NodeJS.ProcessEnv = { + SHELL: "/bin/zsh", + PATH: "/usr/bin", + SSH_AUTH_SOCK: "/tmp/inherited.sock", + }; + const readEnvironment = vi.fn(() => { + throw new Error("probe failed"); + }); + + expect(() => + syncShellEnvironment(env, { + platform: "darwin", + readEnvironment, + }), + ).not.toThrow(); + expect(env.PATH).toBe("/usr/bin"); + expect(env.SSH_AUTH_SOCK).toBe("/tmp/inherited.sock"); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/syncShellEnvironment.test.ts` around lines 5 - 85, Add a test that verifies syncShellEnvironment leaves the inherited env untouched when the login-shell probe throws: create a new "it" case that sets platform: "darwin", an env with PATH and SSH_AUTH_SOCK, and a readEnvironment mock that throws (e.g. vi.fn(() => { throw new Error('probe failed') }) or vi.fn().mockImplementation(() => { throw ... })), call syncShellEnvironment(env, { platform: "darwin", readEnvironment }), and assert that readEnvironment was called but env.PATH and env.SSH_AUTH_SOCK remain unchanged and no exception bubbles out; reference the syncShellEnvironment function and the readEnvironment mock in your test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/syncShellEnvironment.ts`:
- Around line 13-14: Treat empty or whitespace-only SHELL as missing: replace
the current shell selection so it trims env.SHELL and falls back to "/bin/zsh"
when env.SHELL is undefined, null, empty or only whitespace before calling
(options.readEnvironment ?? readEnvironmentFromLoginShell)(...), e.g. compute
shell via a trimmed truthy check (use env.SHELL?.trim() or equivalent) and pass
that value to readEnvironmentFromLoginShell; update references to the local
variable name (shell) used in the call to ensure blank values no longer attempt
to exec an unusable path.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 282-289: The GitHub App token step (id: app_token, uses:
actions/create-github-app-token@v2) is unscoped; update the step to explicitly
restrict the token to the current repository and least-privilege permissions by
adding the repository input set to the current repo (use the github.repository
expression) and a permissions block that at minimum sets contents: write (and
any other specific minimal permissions required), so the token no longer
inherits broad installation defaults.
In `@apps/desktop/src/syncShellEnvironment.test.ts`:
- Around line 5-85: Add a test that verifies syncShellEnvironment leaves the
inherited env untouched when the login-shell probe throws: create a new "it"
case that sets platform: "darwin", an env with PATH and SSH_AUTH_SOCK, and a
readEnvironment mock that throws (e.g. vi.fn(() => { throw new Error('probe
failed') }) or vi.fn().mockImplementation(() => { throw ... })), call
syncShellEnvironment(env, { platform: "darwin", readEnvironment }), and assert
that readEnvironment was called but env.PATH and env.SSH_AUTH_SOCK remain
unchanged and no exception bubbles out; reference the syncShellEnvironment
function and the readEnvironment mock in your test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 726662b5-7cbf-408b-9ca2-388da39f4266
📒 Files selected for processing (13)
.github/workflows/release.ymlapps/desktop/src/fixPath.tsapps/desktop/src/main.tsapps/desktop/src/syncShellEnvironment.test.tsapps/desktop/src/syncShellEnvironment.tsapps/web/public/mockServiceWorker.jsapps/web/src/components/ChatView.tsxapps/web/src/components/Sidebar.logic.tsapps/web/src/components/Sidebar.tsxapps/web/src/components/ui/sidebar.test.tsxapps/web/src/components/ui/sidebar.tsxpackages/shared/src/shell.test.tspackages/shared/src/shell.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/fixPath.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cherry-picks 6 upstream commits (bug fixes and CI improvements):
Intentionally excludes pingdotgg#1171 (configurable git text gen model) and pingdotgg#1032 (terminal context for agents) — those are larger features to integrate separately.
Test plan
Summary by CodeRabbit
New Features
Style
Chores
Tests