fix: keep only the latest value per key in CLAUDE_ENV_FILE (SessionStart) - #748
fix: keep only the latest value per key in CLAUDE_ENV_FILE (SessionStart)#748MeGaNeKoS wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3046ee550c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
*
*
7 Eyl 2026 Pzt 14:30 tarihinde めがねこ ***@***.***> şunu yazdı:
… ***@***.**** commented on this pull request.
------------------------------
In plugins/codex/scripts/session-lifecycle-hook.mjs
<#748 (comment)>
:
> + if (fs.readFileSync(process.env.CLAUDE_ENV_FILE, "utf8").includes(line)) {
+ return;
CLAUDE_ENV_FILE only set once.
—
Reply to this email directly, view it on GitHub
<#748?email_source=notifications&email_token=B3WU6VV5PGJILAXWDGSF6QT5N2L5ZA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJTGE2DMOJRGA32M4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#discussion_r3949290453>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/B3WU6VRQLK6BMDPLSYL63IT5N2L5ZAVCNFSNUABGKJSXA33TNF2G64TZHMYTCOJWGMZDCMJWGI5US43TOVSTWNJTGY4TGNBUGYYTNILWAI>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/B3WU6VWVRGS4RIZ65427TPT5N2L5ZA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJTGE2DMOJRGA32M4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSVGM33PORSXEX3JN5ZQ>
and Android
<https://github.com/notifications/mobile/android/B3WU6VVOMB3TVMORQCIH5ZL5N2L5ZA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJTGE2DMOJRGA32M4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSXGM33PORSXEX3BNZSHE33JMQ>.
Download it today!
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
3046ee5 to
4ab1fa2
Compare
MeGaNeKoS
changed the title
|
Switching this from "append, skip if the line already exists" to set semantics, and it's worth saying why, since the append version looks simpler. The env file is sourced, so the effective value of a key is whatever its last
Tradeoff, to be upfront: set does a read plus a full rewrite per call instead of one append. That's negligible here, the file is well under 1 KB and SessionStart is an occasional event (startup/resume/clear/compact), not a hot path. On Windows, rename over an open file can rarely throw EPERM in that same narrow double-fire window; happy to add a small retry if you'd prefer. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ab1fa24bd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const tmp = `${envFile}.${process.pid}.tmp`; | ||
| fs.writeFileSync(tmp, lines.join("\n") + "\n", "utf8"); | ||
| fs.renameSync(tmp, envFile); |
There was a problem hiding this comment.
When another matching SessionStart hook writes to the shared CLAUDE_ENV_FILE after this read, the subsequent whole-file rename silently discards that hook's export. This can occur in installations with additional user or plugin SessionStart hooks, leaving later commands without their persisted environment variables; use an append-based update or otherwise serialize read-modify-write access.
Useful? React with 👍 / 👎.
| const tmp = `${envFile}.${process.pid}.tmp`; | ||
| fs.writeFileSync(tmp, lines.join("\n") + "\n", "utf8"); | ||
| fs.renameSync(tmp, envFile); |
There was a problem hiding this comment.
When Claude created the existing environment file with a restrictive mode such as 0600 and its parent directory is traversable, writeFileSync creates the replacement using the default 0666 & umask mode—commonly 0644—and the rename installs those broader permissions. Because this file may contain exports from other hooks, including credentials, replacing it should preserve the original mode or explicitly create the temporary file with equally restrictive permissions.
Useful? React with 👍 / 👎.
|
*
*
7 Eyl 2026 Pzt 15:22 tarihinde chatgpt-codex-connector[bot] <
***@***.***> şunu yazdı:
… ***@***.***[bot]* commented on this pull request.
💡 Codex Review
Here are some automated review suggestions for this pull request.
*Reviewed commit:* 4ab1fa2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo.
Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment ***@***.*** <https://github.com/codex> review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT <https://openai.com/codex>,
Codex can also answer questions or update the PR, like ***@***.***
<https://github.com/codex> address that feedback".
------------------------------
In plugins/codex/scripts/session-lifecycle-hook.mjs
<#748 (comment)>
:
> + const tmp = `${envFile}.${process.pid}.tmp`;
+ fs.writeFileSync(tmp, lines.join("\n") + "\n", "utf8");
+ fs.renameSync(tmp, envFile);
*[image: P1 Badge]
<https://camo.githubusercontent.com/c595229c0ecb6ee85b9c7804144d495f131a495ec87091fea2b262d954c9a92d/68747470733a2f2f696d672e736869656c64732e696f2f62616467652f50312d6f72616e67653f7374796c653d666c6174>
Avoid overwriting concurrent SessionStart hook updates*
When another matching SessionStart hook writes to the shared
CLAUDE_ENV_FILE after this read, the subsequent whole-file rename
silently discards that hook's export. This can occur in installations with
additional user or plugin SessionStart hooks, leaving later commands
without their persisted environment variables; use an append-based update
or otherwise serialize read-modify-write access.
Useful? React with 👍 / 👎.
------------------------------
In plugins/codex/scripts/session-lifecycle-hook.mjs
<#748 (comment)>
:
> + const tmp = `${envFile}.${process.pid}.tmp`;
+ fs.writeFileSync(tmp, lines.join("\n") + "\n", "utf8");
+ fs.renameSync(tmp, envFile);
*[image: P2 Badge]
<https://camo.githubusercontent.com/f2c1aacb361ddd3a0e9f9cacdb84fab050de434017f6747bb916e31e29bdf03d/68747470733a2f2f696d672e736869656c64732e696f2f62616467652f50322d79656c6c6f773f7374796c653d666c6174>
Preserve restrictive permissions on the environment file*
When Claude created the existing environment file with a restrictive mode
such as 0600 and its parent directory is traversable, writeFileSync
creates the replacement using the default 0666 & umask mode—commonly 0644—and
the rename installs those broader permissions. Because this file may
contain exports from other hooks, including credentials, replacing it
should preserve the original mode or explicitly create the temporary file
with equally restrictive permissions.
Useful? React with 👍 / 👎.
—
Reply to this email directly, view it on GitHub
<#748?email_source=notifications&email_token=B3WU6VWT3AHNJTNDWIYQZXL5N2SAXA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJTGE4TENJVGU2KM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-5131925554>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/B3WU6VXT6FSPYLU6ZC47F2T5N2SAXAVCNFSNUABGKJSXA33TNF2G64TZHMYTCOJWGMZDCMJWGI5US43TOVSTWNJTGY4TGNBUGYYTNILWAI>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/B3WU6VWP563JIUTVFD4QVWD5N2SAXA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJTGE4TENJVGU2KM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/B3WU6VV5AZSXQGBCD2EDEFT5N2SAXA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMJTGE4TENJVGU2KM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you commented.Message ID:
***@***.***>
|
2 participants
Problem
handleSessionStartcallsappendEnvVarforCODEX_COMPANION_SESSION_ID,CODEX_COMPANION_TRANSCRIPT_PATH, andCLAUDE_PLUGIN_DATAon every fire.hooks/hooks.jsonregisters SessionStart with no matcher, so it runs on startup, resume, /clear, and compact. Those three values are constant for the life of a session, so every fire after the first re-appends bytes already inCLAUDE_ENV_FILE. The file grows without bound.Claude Code inlines that file into the
bash -cpreamble of every Bash tool call, so past a size threshold every command in the session breaks. On Windows (Git Bash) I saw one env file reach 1248 lines / 130 KB containing only 3 unique lines, with three failure shapes by size:unexpected EOF while looking for matching 'ENAMETOOLONG ... uv_spawnQuote escaping in
shellEscapeis correct; the bug is purely the unbounded append.Fix
Replace the unconditional append with set semantics.
setEnvremoves any existingexport <name>=line and writes the current value, so each key appears once with its latest value. This bounds the file and, unlike dedup-by-line, correctly handles a key returning to an earlier value (A -> B -> A ends at A, not a stale B). The write goes through a temp file plus rename so a concurrent SessionStart can't read a half-written file.Test
Repeated identical fires keep the file at one line per key; a fire with a changed value replaces the line instead of appending.