fix(session-lifecycle): make appendEnvVar idempotent (#322)#386
fix(session-lifecycle): make appendEnvVar idempotent (#322)#386NYCU-Chung 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: 31fba86c2f
ℹ️ 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".
| fs.readFileSync(process.env.CLAUDE_ENV_FILE, "utf8").includes(`export ${name}=`) | ||
| ) { | ||
| return; |
There was a problem hiding this comment.
When CLAUDE_ENV_FILE is reused after switching/resuming to a different Claude session, it can already contain export CODEX_COMPANION_SESSION_ID=... for the previous session; this check then returns before writing the new input.session_id. Subsequent Bash commands source the stale session id, and the companion commands that filter/tag jobs from CODEX_COMPANION_SESSION_ID can show or create jobs under the wrong session instead of the current one. The dedupe should only skip when the existing export matches the value, or replace the existing line when it differs.
Useful? React with 👍 / 👎.
NYCU-Chung
force-pushed
the
fix/append-env-idempotent
branch
from
a2bdb36 to
860cc24
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 860cc24c02
ℹ️ 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".
| .split("\n") | ||
| .filter((existingLine) => existingLine !== "" && !existingLine.startsWith(`export ${name}=`)); | ||
| kept.push(line); | ||
| fs.writeFileSync(process.env.CLAUDE_ENV_FILE, `${kept.join("\n")}\n`, "utf8"); |
There was a problem hiding this comment.
When a user has another merged SessionStart hook that also writes to CLAUDE_ENV_FILE, this whole-file rewrite can read a snapshot and then overwrite exports appended by that parallel hook before writeFileSync completes, so later Bash commands silently miss the other hook's environment. Claude's hook docs state that “All matching hooks run in parallel” and specifically recommend append for CLAUDE_ENV_FILE to “preserve variables set by other hooks” (https://code.claude.com/docs/en/hooks#persist-environment-variables), so the dedupe needs to avoid truncating/replacing the shared file without locking or an append-only protocol.
Useful? React with 👍 / 👎.
NYCU-Chung
force-pushed
the
fix/append-env-idempotent
branch
from
860cc24 to
6bca0ef
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bca0ef6b8
ℹ️ 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".
| fs.readFileSync(process.env.CLAUDE_ENV_FILE, "utf8").split("\n").includes(line) | ||
| ) { | ||
| return; |
There was a problem hiding this comment.
When CLAUDE_ENV_FILE has already seen a value and then later saw a different one (for example session A appended, then session B appended), resuming session A again makes this includes(line) check return true for the older A export and skip the append, leaving B as the last export in a file that is sourced top-to-bottom. Subsequent commands then inherit the wrong CODEX_COMPANION_SESSION_ID; fresh evidence beyond the prior stale-value review is that the exact-line dedupe still fails when returning to a previously seen value rather than moving that value back to the end.
Useful? React with 👍 / 👎.
Blind fs.appendFileSync accumulates duplicate export lines in CLAUDE_ENV_FILE across SessionStart re-fires (resume/compact). After enough restarts every Bash-tool command is prefixed with N duplicate pairs; on long Windows sessions the bloated env block corrupts the inlined command and the Bash tool returns no output. Stay append-only (the docs require append so parallel SessionStart hooks don't clobber each other) but skip only when the current value is already the effective (last) export for this name; if a different value is last, re-append so the current value wins. The same value re-firing is a no-op, so it cannot accumulate.
NYCU-Chung
force-pushed
the
fix/append-env-idempotent
branch
from
6bca0ef to
530b7f1
Compare
1 participant
appendEnvVar(plugins/codex/scripts/session-lifecycle-hook.mjs) blind-appends to$CLAUDE_ENV_FILEwith no dedup. Claude Code re-runsSessionStarton every resume/compact and reuses the file, so each fire adds anotherexport CODEX_COMPANION_SESSION_ID=…+export CLAUDE_PLUGIN_DATA=…pair; over N restarts the file holds N duplicate pairs. (Reported in #322, closed but still live onmain/ 1.0.4; #339 renames the var but doesn't touch the accumulation.)Impact: on a long Windows Git-Bash session, ~65 compactions grew the file from the normal 0.2 KB (one pair) to ~10 KB (65 pairs), and the bloated env block corrupted the inlined command so the Bash tool returned no output on every command — silent, no error. Truncating the file back to one pair fixed it immediately.
Fix: make
appendEnvVaridempotent — skip the append when the var is already exported in the file. The first fire still writes the pair; subsequent fires are no-ops.