Skip to content

Commit ee6e14f

Browse files
posthog[bot]PostHog Code
andauthored
fix(code): apply SessionStart hook env to UI git/gh commands (#1915)
Co-authored-by: PostHog Code <code@posthog.com>
1 parent 508420d commit ee6e14f

8 files changed

Lines changed: 401 additions & 19 deletions

File tree

apps/code/src/main/services/agent/service.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import type { FsService } from "../fs/service";
5353
import type { McpAppsService } from "../mcp-apps/service";
5454
import type { PosthogPluginService } from "../posthog-plugin/service";
5555
import type { ProcessTrackingService } from "../process-tracking/service";
56+
import { loadSessionEnvOverrides } from "../session-env/loader";
5657
import type { SleepService } from "../sleep/service";
5758
import type { AgentAuthAdapter, McpToolInstallations } from "./auth-adapter";
5859
import { discoverExternalPlugins } from "./discover-plugins";
@@ -983,6 +984,27 @@ When creating pull requests, add the following footer at the end of the PR descr
983984
return taskId ? all.filter((s) => s.taskId === taskId) : all;
984985
}
985986

987+
/**
988+
* Resolve env-var overrides set by the SessionStart-style hooks of the most
989+
* recently active agent session for `taskId`.
990+
*
991+
* Used by git/gh operations triggered from the UI (Commit, Create PR) so
992+
* they pick up the same hook env the agent itself sees — most importantly
993+
* the SSH_AUTH_SOCK that Secretive's hook re-points at the Secretive agent
994+
* for commit signing. Returns an empty object when there is no session for
995+
* the task or when no hook output is available.
996+
*/
997+
public async getSessionEnvForTask(
998+
taskId: string,
999+
): Promise<Record<string, string>> {
1000+
const candidates = this.listSessions(taskId)
1001+
.filter((s) => !!s.config.sessionId)
1002+
.sort((a, b) => b.lastActivityAt - a.lastActivityAt);
1003+
const session = candidates[0];
1004+
if (!session?.config.sessionId) return {};
1005+
return loadSessionEnvOverrides(session.config.sessionId);
1006+
}
1007+
9861008
/**
9871009
* Get sessions that were interrupted for a specific reason.
9881010
* Optionally filter by repoPath to get only sessions for a specific repo.

apps/code/src/main/services/git/service.test.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ vi.mock("../../utils/logger.js", () => ({
2323
},
2424
}));
2525

26+
import type { AgentService } from "../agent/service";
2627
import type { LlmGatewayService } from "../llm-gateway/service";
2728
import type { WorkspaceService } from "../workspace/service";
2829
import { GitService, mapPrState } from "./service";
@@ -32,7 +33,11 @@ describe("GitService.getPrChangedFiles", () => {
3233

3334
beforeEach(() => {
3435
vi.clearAllMocks();
35-
service = new GitService({} as LlmGatewayService, {} as WorkspaceService);
36+
service = new GitService(
37+
{} as LlmGatewayService,
38+
{} as WorkspaceService,
39+
{ getSessionEnvForTask: async () => ({}) } as unknown as AgentService,
40+
);
3641
});
3742

3843
it("flattens paginated GH API results and maps file statuses", async () => {
@@ -140,7 +145,11 @@ describe("GitService.getGhAuthToken", () => {
140145

141146
beforeEach(() => {
142147
vi.clearAllMocks();
143-
service = new GitService({} as LlmGatewayService, {} as WorkspaceService);
148+
service = new GitService(
149+
{} as LlmGatewayService,
150+
{} as WorkspaceService,
151+
{ getSessionEnvForTask: async () => ({}) } as unknown as AgentService,
152+
);
144153
});
145154

146155
it("returns the authenticated GitHub CLI token", async () => {
@@ -198,7 +207,11 @@ describe("GitService.getPrUrlForBranch", () => {
198207

199208
beforeEach(() => {
200209
vi.clearAllMocks();
201-
service = new GitService({} as LlmGatewayService, {} as WorkspaceService);
210+
service = new GitService(
211+
{} as LlmGatewayService,
212+
{} as WorkspaceService,
213+
{ getSessionEnvForTask: async () => ({}) } as unknown as AgentService,
214+
);
202215
});
203216

204217
it("returns the PR URL for a branch via gh pr list", async () => {

apps/code/src/main/services/git/service.ts

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import { inject, injectable } from "inversify";
4141
import { MAIN_TOKENS } from "../../di/tokens";
4242
import { logger } from "../../utils/logger";
4343
import { TypedEventEmitter } from "../../utils/typed-event-emitter";
44+
import type { AgentService } from "../agent/service";
4445
import type { LlmGatewayService } from "../llm-gateway/service";
4546
import type { SidebarPrState } from "../workspace/schemas";
4647
import type { WorkspaceService } from "../workspace/service";
@@ -136,10 +137,31 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
136137
private readonly llmGateway: LlmGatewayService,
137138
@inject(MAIN_TOKENS.WorkspaceService)
138139
private readonly workspaceService: WorkspaceService,
140+
@inject(MAIN_TOKENS.AgentService)
141+
private readonly agentService: AgentService,
139142
) {
140143
super();
141144
}
142145

146+
/**
147+
* Resolve env-var overrides set by the agent's SessionStart hooks for the
148+
* given task. Used so UI-triggered git/gh operations (Commit, Create PR)
149+
* see the same env (notably `SSH_AUTH_SOCK` re-pointed at Secretive) as
150+
* the agent's bash tool. Returns `undefined` if there's nothing to apply.
151+
*/
152+
private async getSessionEnv(
153+
taskId: string | undefined,
154+
): Promise<Record<string, string> | undefined> {
155+
if (!taskId) return undefined;
156+
try {
157+
const env = await this.agentService.getSessionEnvForTask(taskId);
158+
return Object.keys(env).length > 0 ? env : undefined;
159+
} catch (err) {
160+
log.warn("Failed to load session env for task", { taskId, err });
161+
return undefined;
162+
}
163+
}
164+
143165
private async getStateSnapshot(
144166
directoryPath: string,
145167
options?: {
@@ -481,6 +503,7 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
481503
branch?: string,
482504
setUpstream = false,
483505
signal?: AbortSignal,
506+
env?: Record<string, string>,
484507
): Promise<PushOutput> {
485508
const saga = new PushSaga();
486509
const result = await saga.run({
@@ -489,6 +512,7 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
489512
branch: branch || undefined,
490513
setUpstream,
491514
signal,
515+
env,
492516
});
493517
if (!result.success) {
494518
return { success: false, message: result.error };
@@ -538,6 +562,7 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
538562
directoryPath: string,
539563
remote = "origin",
540564
signal?: AbortSignal,
565+
env?: Record<string, string>,
541566
): Promise<PublishOutput> {
542567
const currentBranch = await getCurrentBranch(directoryPath);
543568
if (!currentBranch) {
@@ -550,6 +575,7 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
550575
currentBranch,
551576
true,
552577
signal,
578+
env,
553579
);
554580
return {
555581
success: pushResult.success,
@@ -623,6 +649,8 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
623649
});
624650
};
625651

652+
const sessionEnv = await this.getSessionEnv(input.taskId);
653+
626654
const saga = new CreatePrSaga(
627655
{
628656
getCurrentBranch: (dir) => getCurrentBranch(dir),
@@ -631,14 +659,16 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
631659
getChangedFilesHead: (dir) => this.getChangedFilesHead(dir),
632660
generateCommitMessage: (dir) =>
633661
this.generateCommitMessage(dir, input.conversationContext),
634-
commit: (dir, msg, opts) => this.commit(dir, msg, opts),
662+
commit: (dir, msg, opts) =>
663+
this.commit(dir, msg, { ...opts, envOverride: sessionEnv }),
635664
getSyncStatus: (dir) => this.getGitSyncStatus(dir),
636-
push: (dir) => this.push(dir),
637-
publish: (dir) => this.publish(dir),
665+
push: (dir) =>
666+
this.push(dir, "origin", undefined, false, undefined, sessionEnv),
667+
publish: (dir) => this.publish(dir, "origin", undefined, sessionEnv),
638668
generatePrTitleAndBody: (dir) =>
639669
this.generatePrTitleAndBody(dir, input.conversationContext),
640670
createPr: (dir, title, body, draft) =>
641-
this.createPrViaGh(dir, title, body, draft),
671+
this.createPrViaGh(dir, title, body, draft, sessionEnv),
642672
onProgress: emitProgress,
643673
},
644674
log,
@@ -729,6 +759,8 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
729759
allowEmpty?: boolean;
730760
stagedOnly?: boolean;
731761
taskId?: string;
762+
/** Pre-resolved session env. Internal — used by createPr to avoid re-loading. */
763+
envOverride?: Record<string, string>;
732764
},
733765
): Promise<CommitOutput> {
734766
const fail = (msg: string): CommitOutput => ({
@@ -740,11 +772,15 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
740772

741773
if (!message.trim()) return fail("Commit message is required");
742774

775+
const { envOverride, ...sagaOptions } = options ?? {};
776+
const env = envOverride ?? (await this.getSessionEnv(options?.taskId));
777+
743778
const saga = new CommitSaga();
744779
const result = await saga.run({
745780
baseDir: directoryPath,
746781
message: message.trim(),
747-
...options,
782+
env,
783+
...sagaOptions,
748784
});
749785

750786
if (!result.success) return fail(result.error);
@@ -955,6 +991,7 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
955991
title?: string,
956992
body?: string,
957993
draft?: boolean,
994+
env?: Record<string, string>,
958995
): Promise<{ success: boolean; message: string; prUrl: string | null }> {
959996
const prFooter =
960997
"\n\n---\n*Created with [PostHog Code](https://posthog.com/code?ref=pr)*";
@@ -968,7 +1005,7 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
9681005
}
9691006
if (draft) args.push("--draft");
9701007

971-
const result = await execGh(args, { cwd: directoryPath });
1008+
const result = await execGh(args, { cwd: directoryPath, env });
9721009
if (result.exitCode !== 0) {
9731010
return {
9741011
success: false,
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { promises as fs } from "node:fs";
2+
import os from "node:os";
3+
import path from "node:path";
4+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
5+
import { loadSessionEnvOverrides } from "./loader";
6+
7+
describe("loadSessionEnvOverrides", () => {
8+
const SESSION_ID = "test-session-id";
9+
let configDir: string;
10+
let sessionDir: string;
11+
let originalConfigDir: string | undefined;
12+
13+
beforeEach(async () => {
14+
configDir = await fs.mkdtemp(path.join(os.tmpdir(), "session-env-test-"));
15+
sessionDir = path.join(configDir, "session-env", SESSION_ID);
16+
await fs.mkdir(sessionDir, { recursive: true });
17+
originalConfigDir = process.env.CLAUDE_CONFIG_DIR;
18+
process.env.CLAUDE_CONFIG_DIR = configDir;
19+
});
20+
21+
afterEach(async () => {
22+
if (originalConfigDir === undefined) {
23+
delete process.env.CLAUDE_CONFIG_DIR;
24+
} else {
25+
process.env.CLAUDE_CONFIG_DIR = originalConfigDir;
26+
}
27+
await fs.rm(configDir, { recursive: true, force: true });
28+
});
29+
30+
const writeHook = (name: string, content: string) =>
31+
fs.writeFile(path.join(sessionDir, name), content);
32+
33+
it("returns empty when CLAUDE_CONFIG_DIR is unset", async () => {
34+
delete process.env.CLAUDE_CONFIG_DIR;
35+
expect(await loadSessionEnvOverrides(SESSION_ID)).toEqual({});
36+
});
37+
38+
it("returns empty when session dir does not exist", async () => {
39+
expect(await loadSessionEnvOverrides("missing-session")).toEqual({});
40+
});
41+
42+
it("returns empty when no hook files match", async () => {
43+
await writeHook("ignored.txt", "export FOO=bar\n");
44+
expect(await loadSessionEnvOverrides(SESSION_ID)).toEqual({});
45+
});
46+
47+
it("parses simple export statements from a SessionStart hook", async () => {
48+
await writeHook("sessionstart-hook-0.sh", "export FOO=bar\n");
49+
const overrides = await loadSessionEnvOverrides(SESSION_ID);
50+
expect(overrides.FOO).toBe("bar");
51+
});
52+
53+
it("captures values produced by `printf %q` shell quoting", async () => {
54+
const value = "/Users/alice/Library/foo bar/socket.ssh";
55+
await writeHook(
56+
"sessionstart-hook-0.sh",
57+
`printf 'export SSH_AUTH_SOCK=%q\\n' ${JSON.stringify(value)} | source /dev/stdin\n` +
58+
// also test the expected hook output format directly
59+
`export SSH_AUTH_SOCK='${value}'\n`,
60+
);
61+
const overrides = await loadSessionEnvOverrides(SESSION_ID);
62+
expect(overrides.SSH_AUTH_SOCK).toBe(value);
63+
});
64+
65+
it("merges exports from multiple hook files in sorted order", async () => {
66+
await writeHook("sessionstart-hook-0.sh", "export FIRST=one\n");
67+
await writeHook("sessionstart-hook-1.sh", "export SECOND=two\n");
68+
await writeHook("setup-hook-0.sh", "export THIRD=three\n");
69+
const overrides = await loadSessionEnvOverrides(SESSION_ID);
70+
expect(overrides.FIRST).toBe("one");
71+
expect(overrides.SECOND).toBe("two");
72+
expect(overrides.THIRD).toBe("three");
73+
});
74+
75+
it("ignores files that don't match the SDK hook naming convention", async () => {
76+
await writeHook("setup.sh", "export SHOULD_NOT_LOAD=1\n");
77+
await writeHook("sessionstart-hook-abc.sh", "export ALSO_NO=1\n");
78+
await writeHook("sessionstart-hook-0.sh", "export YES=1\n");
79+
const overrides = await loadSessionEnvOverrides(SESSION_ID);
80+
expect(overrides).toEqual({ YES: "1" });
81+
});
82+
83+
it("does not return vars that already match the parent process env", async () => {
84+
process.env.UNCHANGED_VAR = "same";
85+
await writeHook("sessionstart-hook-0.sh", "export UNCHANGED_VAR=same\n");
86+
try {
87+
const overrides = await loadSessionEnvOverrides(SESSION_ID);
88+
expect(overrides.UNCHANGED_VAR).toBeUndefined();
89+
} finally {
90+
delete process.env.UNCHANGED_VAR;
91+
}
92+
});
93+
94+
it("handles paths with spaces and quotes safely", async () => {
95+
const dirWithSpaces = path.join(configDir, "session-env", "weird id");
96+
await fs.mkdir(dirWithSpaces, { recursive: true });
97+
await fs.writeFile(
98+
path.join(dirWithSpaces, "sessionstart-hook-0.sh"),
99+
"export SPACED=ok\n",
100+
);
101+
const overrides = await loadSessionEnvOverrides("weird id");
102+
expect(overrides.SPACED).toBe("ok");
103+
});
104+
105+
it("returns empty object on bash failure without throwing", async () => {
106+
await writeHook("sessionstart-hook-0.sh", "exit 1\nexport NEVER=set\n");
107+
// sourcing a script that exits cuts the env -0 short, but we should
108+
// gracefully degrade rather than throw.
109+
const overrides = await loadSessionEnvOverrides(SESSION_ID);
110+
expect(overrides.NEVER).toBeUndefined();
111+
});
112+
113+
it("falls back to empty object if bash is missing", async () => {
114+
// Skip this test on systems where bash exists at /bin/bash —
115+
// we only smoke-check that errors are swallowed.
116+
const realPath = process.env.PATH;
117+
process.env.PATH = "";
118+
try {
119+
const overrides = await loadSessionEnvOverrides(SESSION_ID);
120+
// bash may still be found via absolute path; either outcome is fine.
121+
expect(typeof overrides).toBe("object");
122+
} finally {
123+
process.env.PATH = realPath;
124+
}
125+
});
126+
127+
it("does not leak BASH_VERSION or other shell internals", async () => {
128+
await writeHook("sessionstart-hook-0.sh", "export USEFUL=yes\n");
129+
const overrides = await loadSessionEnvOverrides(SESSION_ID);
130+
expect(overrides.BASH_VERSION).toBeUndefined();
131+
expect(overrides.SHLVL).toBeUndefined();
132+
expect(overrides._).toBeUndefined();
133+
expect(overrides.USEFUL).toBe("yes");
134+
});
135+
});

0 commit comments

Comments
 (0)