Skip to content

Fix duplicate SessionStart exports in CLAUDE_ENV_FILE - #668

Open
manavagarwal123 wants to merge 2 commits into
openai:mainfrom
manavagarwal123:fix/sessionstart-env-duplicates
Open

Fix duplicate SessionStart exports in CLAUDE_ENV_FILE#668
manavagarwal123 wants to merge 2 commits into
openai:mainfrom
manavagarwal123:fix/sessionstart-env-duplicates

Conversation

@manavagarwal123

Copy link
Copy Markdown
Contributor

Closes #661

Summary

Prevent duplicate environment variable exports from being appended to CLAUDE_ENV_FILE on every SessionStart event.

Changes

  • Make appendEnvVar() idempotent.
  • Skip writing an export line if it already exists.

Testing

  • Ran the project's test suite.
  • Verified repeated SessionStart events no longer append duplicate export lines.

@manavagarwal123
manavagarwal123 requested a review from a team August 22, 2026 20:19

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 40bf9da8d3

ℹ️ 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".

Comment on lines +43 to +45
const existing = fs.readFileSync(process.env.CLAUDE_ENV_FILE, "utf8");
if (existing.includes(line)) {
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep revisited SessionStart values effective

When values in one CLAUDE_ENV_FILE follow A→B→A, such as when an earlier Claude session is resumed after another start, existing.includes(line) finds the stale first A assignment and returns even though the later B assignment remains effective when the file is sourced. Commands then receive the wrong session ID and transcript path, breaking session-scoped job handling and transferring the wrong transcript; replace/deduplicate assignments by variable, or only skip when the matching assignment is already the last effective one.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! I updated the implementation to remove any existing assignment for the same environment variable before writing the new one. This ensures the last effective assignment always reflects the current SessionStart values and avoids the stale-value case you pointed out.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 77d09cf6fd

ℹ️ 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".

Comment on lines +56 to +59
fs.writeFileSync(
process.env.CLAUDE_ENV_FILE,
lines.join("\n") + "\n",
"utf8"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve concurrent updates to the shared env file

When another SessionStart hook writes to the shared CLAUDE_ENV_FILE after this process reads it but before this call, the full-file overwrite silently discards that hook's exports. Matching hooks can execute concurrently, and this hook performs the vulnerable read-modify-write cycle three times, so users with multiple SessionStart hooks can intermittently lose unrelated environment variables; synchronize the update or retain append-only writes rather than replacing the shared file.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex address that feedback

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SessionStart hook appends duplicate exports to CLAUDE_ENV_FILE until every Bash call fails

1 participant