chore(agents): centralize shared config and refresh guidance#758
chore(agents): centralize shared config and refresh guidance#758
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude review |
![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: 11c90dc85c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
scripts/postinstall.sh
Outdated
| # Docker builds or partial install contexts may run root postinstall without the | ||
| # full source tree. In that case, shared agent files are intentionally absent | ||
| # and syncing local tool shims is not needed. | ||
| if [[ ! -f "scripts/agents/sync-agent-shims.mjs" ]]; then |
There was a problem hiding this comment.
The skip condition only checks for scripts/agents/sync-agent-shims.mjs, so partial install contexts that copy scripts/ but omit dot-directories still execute pnpm run agents:sync and then fail with ENOENT on .agents/config.json. Because this runs during every pnpm install, Docker/layered installs that intentionally exclude .agents/ will break instead of cleanly skipping as intended by the comment above.
Useful? React with 👍 / 👎.
| import { dirname, relative, resolve } from "node:path"; | ||
| import process from "node:process"; | ||
|
|
||
| const repoRoot = resolve(new URL("../..", import.meta.url).pathname); |
There was a problem hiding this comment.
Using new URL("../..", import.meta.url).pathname can produce non-filesystem-safe paths (for example %20 in paths with spaces, and /C:/... style prefixes on Windows). Since repoRoot is used immediately for readFileSync, this can cause agents:sync/agents:check to fail in valid environments. Convert the file URL with fileURLToPath(...) before resolving paths.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import { dirname, relative, resolve } from "node:path"; | ||
| import process from "node:process"; | ||
|
|
||
| const repoRoot = resolve(new URL("../..", import.meta.url).pathname); |
There was a problem hiding this comment.
new URL(...).pathname on Windows returns a path like /C:/Users/... (with a leading slash). When passed to path.resolve, this is treated as a path relative to the current drive root, producing an incorrect doubled path (e.g., D:\C:\Users\...). The correct cross-platform approach is to use fileURLToPath from node:url, which handles drive-letter stripping properly.
| const repoRoot = resolve(new URL("../..", import.meta.url).pathname); | |
| const repoRoot = resolve(fileURLToPath(new URL("../..", import.meta.url))); |
The fileURLToPath import should also be added at the top of the file alongside the other node:url usage:
import { fileURLToPath } from "node:url";![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
No functional bugs found, but this is a substantial infrastructure addition (new postinstall hook, shim sync script, symlink architecture) that introduces non-trivial automation running on every pnpm install — worth a human look at the overall design before merging.
Extended reasoning...
Overview
This PR touches 13 files, adding a new .agents/ canonical config directory, root AGENTS.md/CLAUDE.md symlinks, a 347-line shim sync script (scripts/agents/sync-agent-shims.mjs), bootstrap scripts, a postinstall hook wired into package.json, and updates to .gitignore and CONTRIBUTING.md. No production SDK code is changed.
Security Risks
The postinstall script runs pnpm run agents:sync and pnpm run agents:check on every pnpm install. The guard prevents breakage in partial installs, but the hook unconditionally regenerates local config files and symlinks on every install. The generated .claude/settings.json includes a deny list for secret-bearing files — a positive security addition. No auth, crypto, or production data paths are affected.
Level of Scrutiny
This is tooling/agent infrastructure, not production SDK code, which lowers the risk profile. However, the PR introduces meaningful automation that runs for all contributors on every install: symlink management, file regeneration, and a check-mode exit-code assertion. The design choices here — postinstall always regenerating, the symlink chain CLAUDE.md -> AGENTS.md -> .agents/AGENTS.md, optional codex file handling — are intentional architectural decisions worth human sign-off.
Other Factors
Three nit-level bugs were flagged: a hardcoded pnpm version in setup.sh that creates two sources of truth with package.json; a missing autogenerate header in formatCodexToml() inconsistent with formatCodexEnvironmentToml(); and relative links in .agents/AGENTS.md that resolve incorrectly when accessed via the root symlinks. All are minor and currently non-breaking (versions happen to match), but the pnpm version skew is a latent maintenance hazard. The PR is otherwise well-structured and includes a test script for the bootstrap flow.
scripts/codex/setup.sh
Outdated
| fi | ||
|
|
||
| corepack enable | ||
| corepack prepare [email protected] --activate |
There was a problem hiding this comment.
🟡 scripts/codex/setup.sh line 25 hardcodes corepack prepare [email protected] --activate instead of reading the version from package.json packageManager field, creating two sources of truth with no automated sync. When pnpm is bumped in package.json without updating setup.sh, agent environments will bootstrap with a mismatched pnpm version, potentially causing pnpm install --frozen-lockfile to fail. Fix by reading the version dynamically: pnpm_version=$(node -e "const p=JSON.parse(require("fs").readFileSync("package.json","utf8")); console.log(p.packageManager.split("@")[1].split("+")[0]);") then using corepack prepare pnpm@${pnpm_version} --activate.
Extended reasoning...
What the bug is
Preparing [email protected] for immediate activation...
Scope: all 7 workspace projects
Lockfile is up to date, resolution step is skipped
Progress: resolved 1, reused 0, downloaded 0, added 0
Packages: +720
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Progress: resolved 720, reused 19, downloaded 1, added 0
Progress: resolved 720, reused 19, downloaded 26, added 0
Progress: resolved 720, reused 19, downloaded 56, added 0
Progress: resolved 720, reused 19, downloaded 83, added 0
Progress: resolved 720, reused 19, downloaded 107, added 0
Progress: resolved 720, reused 19, downloaded 121, added 5
Progress: resolved 720, reused 19, downloaded 122, added 18
Progress: resolved 720, reused 19, downloaded 126, added 25
Progress: resolved 720, reused 19, downloaded 128, added 40
Progress: resolved 720, reused 19, downloaded 130, added 54
Progress: resolved 720, reused 19, downloaded 131, added 74
Progress: resolved 720, reused 19, downloaded 143, added 96
Progress: resolved 720, reused 19, downloaded 149, added 103
Progress: resolved 720, reused 19, downloaded 156, added 106
Progress: resolved 720, reused 19, downloaded 169, added 121
Progress: resolved 720, reused 19, downloaded 178, added 130
Progress: resolved 720, reused 19, downloaded 186, added 137
Progress: resolved 720, reused 19, downloaded 194, added 148
Progress: resolved 720, reused 19, downloaded 204, added 160
Progress: resolved 720, reused 19, downloaded 224, added 178
Progress: resolved 720, reused 19, downloaded 241, added 195
Progress: resolved 720, reused 19, downloaded 263, added 218
Progress: resolved 720, reused 19, downloaded 291, added 255
Progress: resolved 720, reused 19, downloaded 316, added 277
Progress: resolved 720, reused 19, downloaded 326, added 292
Progress: resolved 720, reused 19, downloaded 340, added 301
Progress: resolved 720, reused 19, downloaded 360, added 316
Progress: resolved 720, reused 19, downloaded 383, added 342
Progress: resolved 720, reused 19, downloaded 404, added 367
Progress: resolved 720, reused 19, downloaded 437, added 396
Progress: resolved 720, reused 19, downloaded 469, added 420
Progress: resolved 720, reused 19, downloaded 496, added 450
Progress: resolved 720, reused 19, downloaded 535, added 491
Progress: resolved 720, reused 19, downloaded 574, added 532
Progress: resolved 720, reused 19, downloaded 618, added 576
Progress: resolved 720, reused 19, downloaded 640, added 608
Progress: resolved 720, reused 19, downloaded 684, added 643
Progress: resolved 720, reused 19, downloaded 697, added 675
Progress: resolved 720, reused 19, downloaded 697, added 682
Progress: resolved 720, reused 19, downloaded 697, added 687
Progress: resolved 720, reused 19, downloaded 698, added 688
Progress: resolved 720, reused 19, downloaded 698, added 690
Progress: resolved 720, reused 19, downloaded 699, added 691
Progress: resolved 720, reused 19, downloaded 700, added 692
Progress: resolved 720, reused 19, downloaded 700, added 693
Progress: resolved 720, reused 19, downloaded 700, added 695
Progress: resolved 720, reused 19, downloaded 700, added 698
Progress: resolved 720, reused 19, downloaded 700, added 701, done
.../node_modules/protobufjs postinstall$ node scripts/postinstall
.../[email protected]/node_modules/esbuild postinstall$ node install.js
.../node_modules/protobufjs postinstall: Done
.../[email protected]/node_modules/esbuild postinstall: Done
devDependencies:
- @ai-sdk/anthropic 2.0.6
- @ai-sdk/openai 2.0.20
- @langchain/core 1.1.24
- @langchain/langgraph 1.1.4
- @langchain/openai 1.2.7
- @opentelemetry/api 1.9.0
- @opentelemetry/core 2.0.1
- @opentelemetry/resources 2.0.1
- @opentelemetry/sdk-node 0.203.0
- @opentelemetry/sdk-trace-base 2.0.1
- @opentelemetry/sdk-trace-node 2.0.1
- @opentelemetry/semantic-conventions 1.34.0
- @release-it/bumper 7.0.5
- @types/mustache 4.2.6
- @typescript-eslint/eslint-plugin 8.36.0
- @typescript-eslint/parser 8.39.0
- ai 5.0.52
- autoevals 0.0.131
- dotenv 17.2.0
- eslint 9.32.0
- eslint-config-prettier 10.1.8
- eslint-plugin-import 2.32.0
- eslint-plugin-prettier 5.5.4
- happy-dom 20.0.2
- husky 9.1.7
- mustache 4.2.0
- nanoid 5.1.5
- openai 5.10.2
- prettier 3.6.2
- release-it 19.0.4
- tsup 8.5.0
- tsx 4.20.3
- turbo 2.8.13
- typedoc 0.28.9
- typescript 5.8.3
- vitest 3.2.4
- zod 3.25.76
. postinstall$ node -e "const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });"
. postinstall: > [email protected] agents:sync /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs
. postinstall: Updated /home/claude/langfuse-js/.claude/settings.json
. postinstall: Updated /home/claude/langfuse-js/.mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/environments/environment.toml
. postinstall: Updated /home/claude/langfuse-js/.cursor/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.cursor/environment.json
. postinstall: Updated /home/claude/langfuse-js/.vscode/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/config.toml
. postinstall: > [email protected] agents:check /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs --check
. postinstall: Done
. prepare$ husky
. prepare: Done
Done in 52.4s line 25 contains the literal string Preparing [email protected] for immediate activation.... The authoritative pnpm version for the repo is the field in : . These two sources currently agree, but they are independent text that must be kept in sync by hand.
The specific code path
When an agent or developer bootstraps a fresh environment with Preparing [email protected] for immediate activation...
Scope: all 7 workspace projects
Lockfile is up to date, resolution step is skipped
Already up to date
. postinstall$ node -e "const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });"
. postinstall: > [email protected] agents:sync /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs
. postinstall: Updated /home/claude/langfuse-js/.claude/settings.json
. postinstall: Updated /home/claude/langfuse-js/.mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/environments/environment.toml
. postinstall: Updated /home/claude/langfuse-js/.cursor/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.cursor/environment.json
. postinstall: Updated /home/claude/langfuse-js/.vscode/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/config.toml
. postinstall: > [email protected] agents:check /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs --check
. postinstall: Done
. prepare$ husky
. prepare: Done
Done in 3.7s, the script calls Preparing [email protected] for immediate activation... unconditionally (line 25). It does not read . Meanwhile, the script — which generates and validates all other shims from — does not read or write at all. There is no CI gate, no postinstall check, and no generated-file mechanism that would catch a divergence.
Why existing code does not prevent it
The PR adds and scripts and a postinstall hook, all of which validate generated shims against . However, is hand-authored and is not generated from any single source of truth. The only way the two values stay in sync is if the developer remembers to update both files when running Installing [email protected] in the project...
Scope: all 7 workspace projects
Lockfile is up to date, resolution step is skipped
Progress: resolved 1, reused 0, downloaded 0, added 0
Packages: +12
++++++++++++
Progress: resolved 12, reused 0, downloaded 3, added 2
Progress: resolved 12, reused 0, downloaded 8, added 4
Progress: resolved 12, reused 0, downloaded 10, added 8
Progress: resolved 12, reused 0, downloaded 10, added 10
Progress: resolved 12, reused 0, downloaded 12, added 10
Progress: resolved 12, reused 0, downloaded 12, added 12, done
devDependencies:
- @langchain/core 1.1.24
- @langchain/langgraph 1.1.4
- @langchain/openai 1.2.7
- @typescript-eslint/eslint-plugin 8.36.0
- eslint-plugin-import 2.32.0
- eslint-plugin-prettier 5.5.4
. postinstall$ node -e "const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });"
. postinstall: > [email protected] agents:sync /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs
. postinstall: Updated /home/claude/langfuse-js/.claude/settings.json
. postinstall: Updated /home/claude/langfuse-js/.mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/environments/environment.toml
. postinstall: Updated /home/claude/langfuse-js/.cursor/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.cursor/environment.json
. postinstall: Updated /home/claude/langfuse-js/.vscode/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/config.toml
. postinstall: > [email protected] agents:check /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs --check
. postinstall: Done
. prepare$ husky
. prepare: Done
Done in 8.9s using pnpm v10.33.0 or manually editing .
Impact
If pnpm is bumped in (a routine maintenance step) without updating , any agent or CI job that bootstraps via Preparing [email protected] for immediate activation...
Scope: all 7 workspace projects
Lockfile is up to date, resolution step is skipped
Already up to date
. postinstall$ node -e "const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });"
. postinstall: > [email protected] agents:sync /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs
. postinstall: Updated /home/claude/langfuse-js/.claude/settings.json
. postinstall: Updated /home/claude/langfuse-js/.mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/environments/environment.toml
. postinstall: Updated /home/claude/langfuse-js/.cursor/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.cursor/environment.json
. postinstall: Updated /home/claude/langfuse-js/.vscode/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/config.toml
. postinstall: > [email protected] agents:check /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs --check
. postinstall: Done
. prepare$ husky
. prepare: Done
Done in 3.4s using pnpm v10.33.0 will call Preparing [email protected]+sha512.10568bb4a6afb58c9eb3630da90cc9516417abebd3fabbe6739f0ae795728da1491e9db5a544c76ad8eb7570f5c4bb3d6c637b2cb41bfdcdb47fa823c8649319... with the old version. Depending on the corepack version and environment: (1) corepack may download and activate the stale release globally, then have Scope: all 7 workspace projects
Lockfile is up to date, resolution step is skipped
Already up to date
. postinstall$ node -e "const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });"
. postinstall: > [email protected] agents:sync /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs
. postinstall: Updated /home/claude/langfuse-js/.claude/settings.json
. postinstall: Updated /home/claude/langfuse-js/.mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/environments/environment.toml
. postinstall: Updated /home/claude/langfuse-js/.cursor/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.cursor/environment.json
. postinstall: Updated /home/claude/langfuse-js/.vscode/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/config.toml
. postinstall: > [email protected] agents:check /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs --check
. postinstall: Done
. prepare$ husky
. prepare: Done
Done in 3.4s using pnpm v10.33.0 fail or warn because the project's field disagrees; (2) it may succeed silently but install with a different pnpm binary than the lockfile was written with, potentially producing unexpected results. One verifier noted that corepack respects per-project even if the global default diverges, which reduces but does not eliminate the practical failure surface — particularly across pnpm versions that changed lockfile format.
How to fix it
Read the version dynamically from before calling Preparing [email protected]+sha512.10568bb4a6afb58c9eb3630da90cc9516417abebd3fabbe6739f0ae795728da1491e9db5a544c76ad8eb7570f5c4bb3d6c637b2cb41bfdcdb47fa823c8649319...:
Preparing pnpm@* for immediate activation...
This removes the duplicate and ensures always activates whatever version the repo declares.
Step-by-step proof of failure
- Developer runs Installing [email protected] in the project...
Internal Error: Server answered with HTTP 404 when performing the request to https://registry.npmjs.org/pnpm/-/pnpm-9.16.0.tgz; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting
at fetch (/opt/node22/lib/node_modules/corepack/dist/lib/corepack.cjs:22096:11)
at process.processTicksAndRejections (node:internal/process/task_queues:103:5)
at async fetchUrlStream (/opt/node22/lib/node_modules/corepack/dist/lib/corepack.cjs:22119:20)
at async download (/opt/node22/lib/node_modules/corepack/dist/lib/corepack.cjs:22290:18)
at async installVersion (/opt/node22/lib/node_modules/corepack/dist/lib/corepack.cjs:22386:55)
at async Engine.ensurePackageManager (/opt/node22/lib/node_modules/corepack/dist/lib/corepack.cjs:22899:32)
at async UseCommand.execute (/opt/node22/lib/node_modules/corepack/dist/lib/corepack.cjs:23524:32)
at async UseCommand.validateAndExecute (/opt/node22/lib/node_modules/corepack/dist/lib/corepack.cjs:20278:22)
at async _Cli.run (/opt/node22/lib/node_modules/corepack/dist/lib/corepack.cjs:21215:18)
at async Object.runMain (/opt/node22/lib/node_modules/corepack/dist/lib/corepack.cjs:23704:19), which updates to . - Developer does NOT update (an easy oversight since the PR adds no lint rule or automated check for this file).
- Agent bootstraps environment: Preparing [email protected] for immediate activation...
Scope: all 7 workspace projects
Lockfile is up to date, resolution step is skipped
Already up to date
. postinstall$ node -e "const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });"
. postinstall: > [email protected] agents:sync /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs
. postinstall: Updated /home/claude/langfuse-js/.claude/settings.json
. postinstall: Updated /home/claude/langfuse-js/.mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/environments/environment.toml
. postinstall: Updated /home/claude/langfuse-js/.cursor/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.cursor/environment.json
. postinstall: Updated /home/claude/langfuse-js/.vscode/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/config.toml
. postinstall: > [email protected] agents:check /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs --check
. postinstall: Done
. prepare$ husky
. prepare: Done
Done in 3.4s using pnpm v10.33.0 calls Preparing [email protected] for immediate activation... (stale).
4. Scope: all 7 workspace projects
Lockfile is up to date, resolution step is skipped
Already up to date
. postinstall$ node -e "const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });"
. postinstall: > [email protected] agents:sync /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs
. postinstall: Updated /home/claude/langfuse-js/.claude/settings.json
. postinstall: Updated /home/claude/langfuse-js/.mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/environments/environment.toml
. postinstall: Updated /home/claude/langfuse-js/.cursor/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.cursor/environment.json
. postinstall: Updated /home/claude/langfuse-js/.vscode/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/config.toml
. postinstall: > [email protected] agents:check /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs --check
. postinstall: Done
. prepare$ husky
. prepare: Done
Done in 3.4s using pnpm v10.33.0 runs with pnpm 9.15.0, but the lockfile was written with 9.16.0 — pnpm may warn or fail if the lockfile format changed across the minor bump.
5. Agent environment is broken or produces unexpected results with no clear error pointing to the version skew.
| const formatCodexToml = () => { | ||
| const lines = []; | ||
|
|
||
| for (const [name, server] of Object.entries(sortObject(servers))) { | ||
| lines.push(`[mcp_servers.${name}]`); | ||
|
|
||
| if (server.transport === "stdio") { | ||
| lines.push(`command = ${JSON.stringify(server.command)}`); |
There was a problem hiding this comment.
🟡 .codex/config.toml is generated by formatCodexToml() but lacks the # THIS IS AUTOGENERATED. DO NOT EDIT MANUALLY header that formatCodexEnvironmentToml() adds to .codex/environments/environment.toml. A developer opening .codex/config.toml without context would not know it is auto-generated and might hand-edit it, only to have their changes silently overwritten on the next pnpm install or pnpm run agents:sync.
Extended reasoning...
What the bug is: (lines 83-122 of ) produces without any autogenerate warning comment. Meanwhile, (lines ~144-156) correctly begins its output with . Both functions generate local TOML config files from , yet only one warns the reader.
The specific code path: starts by initializing an empty array and immediately emits TOML stanzas. There is no guard comment or header. , by contrast, pushes as its very first line.
Why existing mitigations do not fully prevent the issue: The refutation correctly notes that is in and that / document the file as generated. However, those mitigations require a developer to consult external docs first. A developer opening to add an MCP server has no in-file signal that the edit will be discarded. The entry prevents committing the change but does not prevent silent data loss when the next Scope: all 7 workspace projects
Lockfile is up to date, resolution step is skipped
Progress: resolved 1, reused 0, downloaded 0, added 0
Packages: +720
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Progress: resolved 720, reused 0, downloaded 15, added 0
Progress: resolved 720, reused 0, downloaded 30, added 17
Progress: resolved 720, reused 0, downloaded 53, added 33
Progress: resolved 720, reused 0, downloaded 65, added 44
Progress: resolved 720, reused 0, downloaded 80, added 53
Progress: resolved 720, reused 0, downloaded 94, added 65
Progress: resolved 720, reused 0, downloaded 105, added 75
Progress: resolved 720, reused 0, downloaded 123, added 94
Progress: resolved 720, reused 0, downloaded 147, added 108
Progress: resolved 720, reused 0, downloaded 161, added 126
Progress: resolved 720, reused 0, downloaded 193, added 160
Progress: resolved 720, reused 0, downloaded 226, added 195
Progress: resolved 720, reused 0, downloaded 281, added 245
Progress: resolved 720, reused 0, downloaded 321, added 296
Progress: resolved 720, reused 0, downloaded 353, added 326
Progress: resolved 720, reused 0, downloaded 384, added 357
Progress: resolved 720, reused 0, downloaded 423, added 397
Progress: resolved 720, reused 0, downloaded 446, added 421
Progress: resolved 720, reused 0, downloaded 465, added 440
Progress: resolved 720, reused 0, downloaded 494, added 467
Progress: resolved 720, reused 0, downloaded 523, added 501
Progress: resolved 720, reused 0, downloaded 561, added 542
Progress: resolved 720, reused 0, downloaded 593, added 577
Progress: resolved 720, reused 0, downloaded 623, added 601
Progress: resolved 720, reused 0, downloaded 650, added 632
Progress: resolved 720, reused 0, downloaded 672, added 657
Progress: resolved 720, reused 0, downloaded 693, added 671
Progress: resolved 720, reused 0, downloaded 706, added 693
Progress: resolved 720, reused 0, downloaded 706, added 698
Progress: resolved 720, reused 0, downloaded 711, added 701
Progress: resolved 720, reused 0, downloaded 714, added 705
Progress: resolved 720, reused 0, downloaded 714, added 706
Progress: resolved 720, reused 0, downloaded 715, added 707
Progress: resolved 720, reused 0, downloaded 715, added 708
Progress: resolved 720, reused 0, downloaded 715, added 709
Progress: resolved 720, reused 0, downloaded 716, added 709
Progress: resolved 720, reused 0, downloaded 716, added 710
Progress: resolved 720, reused 0, downloaded 717, added 710
Progress: resolved 720, reused 0, downloaded 717, added 711
Progress: resolved 720, reused 0, downloaded 717, added 712
Progress: resolved 720, reused 0, downloaded 718, added 714
Progress: resolved 720, reused 0, downloaded 718, added 715
Progress: resolved 720, reused 0, downloaded 718, added 717
Progress: resolved 720, reused 0, downloaded 719, added 718
Progress: resolved 720, reused 0, downloaded 719, added 719
Progress: resolved 720, reused 0, downloaded 719, added 720
Progress: resolved 720, reused 0, downloaded 719, added 720, done
.../node_modules/protobufjs postinstall$ node scripts/postinstall
.../node_modules/protobufjs postinstall: Done
.../[email protected]/node_modules/esbuild postinstall$ node install.js
.../[email protected]/node_modules/esbuild postinstall: Done
. postinstall$ node -e "const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });"
. postinstall: > [email protected] agents:sync /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs
. postinstall: Updated /home/claude/langfuse-js/.claude/settings.json
. postinstall: Updated /home/claude/langfuse-js/.mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/environments/environment.toml
. postinstall: Updated /home/claude/langfuse-js/.cursor/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.cursor/environment.json
. postinstall: Updated /home/claude/langfuse-js/.vscode/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/config.toml
. postinstall: > [email protected] agents:check /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs --check
. postinstall: Done
. prepare$ husky
. prepare: Done
Done in 55.6s overwrites the file.
Concrete proof: Running Updated /home/claude/langfuse-js/.claude/settings.json
Updated /home/claude/langfuse-js/.mcp.json
Updated /home/claude/langfuse-js/.codex/environments/environment.toml
Updated /home/claude/langfuse-js/.cursor/mcp.json
Updated /home/claude/langfuse-js/.cursor/environment.json
Updated /home/claude/langfuse-js/.vscode/mcp.json
Updated /home/claude/langfuse-js/.codex/config.toml with the current config produces starting with and starting with . Opening both files side by side clearly shows the inconsistency.
Impact: Developer opens , edits it, runs Scope: all 7 workspace projects
Lockfile is up to date, resolution step is skipped
Already up to date
. postinstall$ node -e "const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });"
. postinstall: > [email protected] agents:sync /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs
. postinstall: Updated /home/claude/langfuse-js/.claude/settings.json
. postinstall: Updated /home/claude/langfuse-js/.mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/environments/environment.toml
. postinstall: Updated /home/claude/langfuse-js/.cursor/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.cursor/environment.json
. postinstall: Updated /home/claude/langfuse-js/.vscode/mcp.json
. postinstall: Updated /home/claude/langfuse-js/.codex/config.toml
. postinstall: > [email protected] agents:check /home/claude/langfuse-js
. postinstall: > node scripts/agents/sync-agent-shims.mjs --check
. postinstall: Done
. prepare$ husky
. prepare: Done
Done in 3.7s or
[email protected] agents:sync /home/claude/langfuse-js
node scripts/agents/sync-agent-shims.mjs
Updated /home/claude/langfuse-js/.claude/settings.json
Updated /home/claude/langfuse-js/.mcp.json
Updated /home/claude/langfuse-js/.codex/environments/environment.toml
Updated /home/claude/langfuse-js/.cursor/mcp.json
Updated /home/claude/langfuse-js/.cursor/environment.json
Updated /home/claude/langfuse-js/.vscode/mcp.json
Updated /home/claude/langfuse-js/.codex/config.toml later, and loses the changes silently with no in-file warning to edit instead.
Fix: Add as the first line pushed in , matching the pattern established in .
.agents/AGENTS.md
Outdated
| - Shared agent setup overview: [`README.md`](README.md) | ||
| - Shared skill index: [`skills/README.md`](skills/README.md) | ||
| - This repo does not currently define shared skills beyond the index. Add them |
There was a problem hiding this comment.
🟡 The relative links README.md and skills/README.md in .agents/AGENTS.md (lines 151-152) resolve correctly when reading the canonical file directly, but break when accessed via the root AGENTS.md or CLAUDE.md symlinks — README.md points to the project root README (wrong file) and skills/README.md points to a non-existent path. Update them to .agents/README.md and .agents/skills/README.md to match the style already used in CONTRIBUTING.md.
Extended reasoning...
What the bug is
In .agents/AGENTS.md, the Agent-specific Notes section contains two relative markdown links at lines 151-152: [README.md](README.md) and [skills/README.md](skills/README.md). These are written relative to the .agents/ directory where the canonical file lives, so they resolve correctly when reading .agents/AGENTS.md directly.
How it manifests via symlinks
This PR introduces two root-level symlinks: AGENTS.md pointing to .agents/AGENTS.md and CLAUDE.md pointing to AGENTS.md. The diff confirms these are git symlink files whose content is literally the target path string. When a markdown renderer, or any tool that follows symlinks but resolves relative paths from the symlink directory, processes AGENTS.md or CLAUDE.md, it resolves relative links from the repo root rather than from .agents/. The result: README.md resolves to the project root README.md (the main project README, wrong file), and skills/README.md resolves to ./skills/README.md in the repo root, which does not exist.
Why existing code does not prevent it
The file was written with the assumption it would only be read at .agents/AGENTS.md. The symlink indirection was introduced in this same PR without updating the link targets to account for the new resolution context. There is no runtime validation of markdown link correctness.
Impact
Anyone viewing AGENTS.md or CLAUDE.md on GitHub or in a local markdown preview tool would see a wrong link for the setup overview and a dead link for the skills index. The practical impact is limited since AI agents read file content rather than following hyperlinks, but human contributors navigating the docs would be misled. The inconsistency with CONTRIBUTING.md, which already uses the correct .agents/README.md and .agents/skills/README.md forms in this same PR, makes the codebase harder to maintain.
Step-by-step proof
AGENTS.mdat the repo root is a symlink: its content in the diff is the single line.agents/AGENTS.md.- When GitHub renders
AGENTS.md, it follows the symlink but resolves relative link paths from the repo root where the symlink lives. - The link
[README.md](README.md)is resolved as<repo-root>/README.md, which is the main project README, not.agents/README.md. - The link
[skills/README.md](skills/README.md)is resolved as<repo-root>/skills/README.md, a path that does not exist. CONTRIBUTING.mdupdated in this same PR already writes these correctly as.agents/README.mdand.agents/skills/README.md, confirming the intended style.
How to fix
Change the two link targets in .agents/AGENTS.md lines 151-152 to use root-relative paths: .agents/README.md and .agents/skills/README.md. This makes them work correctly from both the canonical .agents/AGENTS.md location and the root symlink context.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09c67a2a90
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "ci": "pnpm build && pnpm test && pnpm lint && pnpm typecheck && pnpm format:check", | ||
| "clean": "pnpm -r run clean && rm -rf packages/*/dist && rm -f packages/*/tsconfig.tsbuildinfo", | ||
| "nuke": "pnpm clean && rm -rf node_modules && rm -rf packages/*/node_modules && pnpm install", | ||
| "postinstall": "node -e \"const fs = require('node:fs'); const cp = require('node:child_process'); if (!fs.existsSync('scripts/postinstall.sh')) { console.log('Skipping repo postinstall helper: scripts/postinstall.sh is not present in this install context.'); process.exit(0); } cp.execSync('bash scripts/postinstall.sh', { stdio: 'inherit' });\"", |
There was a problem hiding this comment.
The new postinstall script always invokes cp.execSync('bash scripts/postinstall.sh', ...), which makes pnpm install fail in environments that don't have bash on PATH (notably default Windows cmd/PowerShell setups). Because this runs on every install, contributors in those environments cannot bootstrap dependencies at all even when the rest of the repo is Node-based.
Useful? React with 👍 / 👎.
| symlinkSync( | ||
| relative(dirname(output.path), output.target), | ||
| output.path, | ||
| lstatSync(output.target).isDirectory() ? "dir" : "file", | ||
| ); |
There was a problem hiding this comment.
agents:sync unconditionally replaces outputs with symlinkSync(...) for AGENTS.md/CLAUDE.md and skill links; if symlink creation is not permitted (common on Windows without symlink privileges), this throws and causes install-time failure because postinstall runs agents:sync. A fallback path is needed so environments that cannot create symlinks still complete installation.
Useful? React with 👍 / 👎.
1 participant
Problem
.agents/Changes
.agents/as the canonical source of truth for shared repo agent guidance and configAGENTS.mdandCLAUDE.mdcompatibility symlinksscripts/agents/sync-agent-shims.mjsplusagents:sync/agents:checkandpostinstall.cursor/mcp.jsonand.cursor/environment.jsonAGENTS.mdwith repo-specific tooling, CI, release, verification, and instruction-writing guidanceExternal Guidance Reviewed
Verification
node scripts/agents/sync-agent-shims.mjsnode scripts/agents/sync-agent-shims.mjs --checkbash scripts/codex/setup.test.shbash -n scripts/postinstall.sh scripts/codex/setup.sh scripts/codex/setup.test.shDisclaimer: Experimental PR review
Greptile Summary
This PR centralizes shared agent configuration and guidance under
.agents/as a single source of truth, introduces async-agent-shims.mjsscript to generate tool-specific config files (Claude, Cursor, VS Code, Codex) from that shared config onpnpm install, and wires everything together with apostinstallhook andagents:sync/agents:checknpm scripts.\n\nKey changes:\n-.agents/— New canonical directory containingAGENTS.md(shared repo guidance),config.json(MCP server catalog, Claude permissions, Cursor/Codex environment inputs), and askills/index\n- Root symlinks —AGENTS.md→.agents/AGENTS.md,CLAUDE.md→AGENTS.md(two-level chain; tools that follow symlinks will resolve to the canonical content)\n-sync-agent-shims.mjs— Generates all tool-specific shims on demand; also used in--checkmode to enforce drift detection in CI. One Windows portability bug: usesnew URL(...).pathnameinstead offileURLToPath(new URL(...))which produces an incorrect double-drive-letter path on Windows\n-postinstallhook — Guards against partial install contexts (Docker, package-only installs) before delegating toscripts/postinstall.sh; correctly avoids infinite recursion since the helpers don't re-invokepnpm install\n-scripts/codex/setup.sh— Hardcodes[email protected], which matchespackageManagertoday but will silently drift if only one of the two is updated\n-.gitignore— All generated shim paths are correctly excluded from source controlConfidence Score: 4/5
Safe to merge on Linux/macOS CI; one targeted fix recommended before the sync script is relied upon on Windows.
The overall architecture is well-thought-out: centralized config, deterministic shim generation, --check mode for CI drift detection, and graceful postinstall skipping are all solid. The one concrete bug is the Windows-incompatible new URL(...).pathname in sync-agent-shims.mjs — swapping in fileURLToPath is a one-liner fix. The remaining comments are P2 style/maintenance suggestions. Since the repo CI runs on Linux, the Windows issue won't block merging, but it is worth fixing before contributors on Windows try to use the tooling.
scripts/agents/sync-agent-shims.mjs — Windows path extraction bug; scripts/codex/setup.sh — hardcoded pnpm version drift risk.
Important Files Changed
.agents/config.json; has a Windows-incompatible file URL path extraction (pathnameinstead offileURLToPath) and overly broad error swallowing for optional outputs.packageManagerfield inpackage.jsonif one is updated without the other.agents:sync,agents:checkscripts and apostinstallhook that guards against missing-script contexts and delegates toscripts/postinstall.sh; no issues found.setup.shusing mockcorepack/pnpmbinaries; correctly verifies.envcreation from example and frozen-lockfile install invocation.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[pnpm install] --> B[postinstall lifecycle hook] B --> C{scripts/postinstall.sh present?} C -- No --> D[Skip with warning] C -- Yes --> E[scripts/postinstall.sh] E --> F[pnpm run agents:sync] E --> G[pnpm run agents:check] F --> H[sync-agent-shims.mjs] G --> I[sync-agent-shims.mjs --check] H --> J[Read .agents/config.json] J --> K[Write .claude/settings.json] J --> L[Write .mcp.json] J --> M[Write .cursor/mcp.json + environment.json] J --> N[Write .vscode/mcp.json] J --> O[Write .codex/config.toml + environment.toml - optional] J --> P[Create symlinks: AGENTS.md and CLAUDE.md] I --> Q{All files match?} Q -- Yes --> R[Exit 0] Q -- No --> S[Exit 1]Reviews (1): Last reviewed commit: "chore(agents): centralize shared config ..." | Re-trigger Greptile