fix(code): apply SessionStart hook env to UI git/gh commands#1915
Conversation
The Commit and Create PR buttons run git/gh in the main process via `simple-git` and `execGh`, both of which were inheriting bare `process.env`. That ignored the env updates the Claude Agent SDK applies for each session via SessionStart-style hooks (which the SDK writes as `<event>-hook-N.sh` files under `<CLAUDE_CONFIG_DIR>/session-env/<sessionId>/` and sources before its own bash tool commands). Most visibly, when a user has the Secretive code-signing hook installed and launches PostHog Code from the Dock (so the parent process inherits the default macOS launchd `SSH_AUTH_SOCK`), the agent can sign commits but the UI buttons cannot — the same `SSH_AUTH_SOCK` repoint never reached the git subprocess they spawn. Mirror the SDK's behavior in the main process: - New `loadSessionEnvOverrides(sessionId)` util sources the SDK's hook files via bash and returns the env diff (skipping shell internals). - `AgentService.getSessionEnvForTask(taskId)` resolves the most recent active session for a task and returns its overrides. - Plumb an optional `env` through `@posthog/git` (`execGh`, `GitSagaInput`, `ExecuteOptions`) so it reaches both the simple-git client and `gh`. - `GitService.commit` and `createPr` look up the session env when a `taskId` is present and thread it through commit / push / publish / `gh pr create`. Generated-By: PostHog Code Task-Id: 86938233-5c7d-4285-b996-6bd9cf09b57c
405ea77 to
e259eb5
Compare
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/git/src/operation-manager.ts:97-101
`GIT_OPTIONAL_LOCKS: "0"` is set before `...options?.env`, so any session hook that exports `GIT_OPTIONAL_LOCKS` (or even an innocuous hook that happens to re-export it as part of its full env dump) would silently override the `"0"` for every read operation. Since `BASH_INTERNAL_VARS` does not filter git-specific vars, this is a real exposure. The constant should be the last write so it always wins for reads.
```suggestion
const env = {
...getCleanEnv(),
...options?.env,
GIT_OPTIONAL_LOCKS: "0",
};
```
### Issue 2 of 2
apps/code/src/main/services/agent/service.ts:1000-1004
The `.filter((s) => !!s.config.sessionId)` guarantees every item in `candidates` has a truthy `sessionId`, making the subsequent `if (!session?.config.sessionId)` branch dead code — it can only be reached when `candidates` is empty (i.e. `session` is `undefined`). Simplify to a plain presence check.
```suggestion
const candidates = this.listSessions(taskId)
.filter((s) => !!s.config.sessionId)
.sort((a, b) => b.lastActivityAt - a.lastActivityAt);
const session = candidates[0];
if (!session) return {};
```
Reviews (1): Last reviewed commit: "Merge branch 'main' into posthog-code/re..." | Re-trigger Greptile |
| const env = { | ||
| ...getCleanEnv(), | ||
| GIT_OPTIONAL_LOCKS: "0", | ||
| ...options?.env, | ||
| }; |
There was a problem hiding this comment.
GIT_OPTIONAL_LOCKS: "0" is set before ...options?.env, so any session hook that exports GIT_OPTIONAL_LOCKS (or even an innocuous hook that happens to re-export it as part of its full env dump) would silently override the "0" for every read operation. Since BASH_INTERNAL_VARS does not filter git-specific vars, this is a real exposure. The constant should be the last write so it always wins for reads.
| const env = { | |
| ...getCleanEnv(), | |
| GIT_OPTIONAL_LOCKS: "0", | |
| ...options?.env, | |
| }; | |
| const env = { | |
| ...getCleanEnv(), | |
| ...options?.env, | |
| GIT_OPTIONAL_LOCKS: "0", | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/git/src/operation-manager.ts
Line: 97-101
Comment:
`GIT_OPTIONAL_LOCKS: "0"` is set before `...options?.env`, so any session hook that exports `GIT_OPTIONAL_LOCKS` (or even an innocuous hook that happens to re-export it as part of its full env dump) would silently override the `"0"` for every read operation. Since `BASH_INTERNAL_VARS` does not filter git-specific vars, this is a real exposure. The constant should be the last write so it always wins for reads.
```suggestion
const env = {
...getCleanEnv(),
...options?.env,
GIT_OPTIONAL_LOCKS: "0",
};
```
How can I resolve this? If you propose a fix, please make it concise.| const candidates = this.listSessions(taskId) | ||
| .filter((s) => !!s.config.sessionId) | ||
| .sort((a, b) => b.lastActivityAt - a.lastActivityAt); | ||
| const session = candidates[0]; | ||
| if (!session?.config.sessionId) return {}; |
There was a problem hiding this comment.
The
.filter((s) => !!s.config.sessionId) guarantees every item in candidates has a truthy sessionId, making the subsequent if (!session?.config.sessionId) branch dead code — it can only be reached when candidates is empty (i.e. session is undefined). Simplify to a plain presence check.
| const candidates = this.listSessions(taskId) | |
| .filter((s) => !!s.config.sessionId) | |
| .sort((a, b) => b.lastActivityAt - a.lastActivityAt); | |
| const session = candidates[0]; | |
| if (!session?.config.sessionId) return {}; | |
| const candidates = this.listSessions(taskId) | |
| .filter((s) => !!s.config.sessionId) | |
| .sort((a, b) => b.lastActivityAt - a.lastActivityAt); | |
| const session = candidates[0]; | |
| if (!session) return {}; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/main/services/agent/service.ts
Line: 1000-1004
Comment:
The `.filter((s) => !!s.config.sessionId)` guarantees every item in `candidates` has a truthy `sessionId`, making the subsequent `if (!session?.config.sessionId)` branch dead code — it can only be reached when `candidates` is empty (i.e. `session` is `undefined`). Simplify to a plain presence check.
```suggestion
const candidates = this.listSessions(taskId)
.filter((s) => !!s.config.sessionId)
.sort((a, b) => b.lastActivityAt - a.lastActivityAt);
const session = candidates[0];
if (!session) return {};
```
How can I resolve this? If you propose a fix, please make it concise.
Problem
The Commit and Create PR buttons run git/gh from the main process (via
simple-gitandexecGh), and both were inheriting bareprocess.env. That ignored the env updates the Claude Agent SDK applies per session through SessionStart-style hooks — the SDK writes each hook's output to<CLAUDE_CONFIG_DIR>/session-env/<sessionId>/<event>-hook-<N>.shand sources those files before each of its own bash tool commands.Most visibly: when a user has the Secretive code-signing hook installed and launches PostHog Code from the Dock (so the parent inherits the default macOS launchd `SSH_AUTH_SOCK`), the agent can sign commits but the UI Commit / Create PR buttons cannot — the `SSH_AUTH_SOCK` repoint the hook performs never reaches the git subprocess they spawn.
Changes
Mirror the SDK's session-env behavior in the main process so UI-triggered git/gh sees the same env the agent does:
Test plan
Created with PostHog Code