Repository navigation
fix(config): merge custom toolPaths tool by tool over defaults (#1185) - #1204
Conversation
No other confident findings in the specified diff. No PR code was run, built, or installed. |
|
Resolved in commit 7a7cc4f. Added a dedicated real-CLI end-to-end test in src/tests/e2e/toolpaths-sparse-merge-1185.test.ts exercising the compiled CLI artifact (dist/index.js) against a local Git bare remote fixture configured with sparse oolPaths (zcode: { skills: '.zcode/skills', rules: '.zcode/rules' }). Representative real-CLI execution verifies that:
|
|
Flagging a possible conflict with maintainer direction before this merges (Triage access only, not weighing in on which approach is right — that's a call for @jeff-r2026). This PR's fix for #1185 changes On #1115 — the same root mechanism, different symptom — @jeff-r2026 was explicit:
PR #1119 (for #1115) follows that: doctor-only warning, no schema change. This PR goes the other way at the schema level. If both land, #1119's "replace-whole, just warn about it" premise and this PR's "merge tool-by-tool so nothing is silently dropped" premise would be solving overlapping problems with opposite designs — worth a maintainer call on which one should actually ship before either merges, rather than discovering the conflict after the fact. |
|
No confident findings in the specified diff. The earlier [P1 blocking] Missing real-CLI verification record finding is resolved: the PR description now documents a build followed by representative No PR code was run, built, or installed. |
|
@STiFLeR7 I see the conflict with Jeff's earlier direction. My preference as a contributor is the tool-by-tool merge proposed here. A team that customizes CodeBuddy's paths should still retain the default paths for Claude, Codex, and other tools. Otherwise, customizing one tool requires the team to copy and maintain the entire default table. This PR preserves whole-entry replacement for the named tool. Omitting If a team wants to block This changes existing behavior for teams that used |
|
@legend91019 CI is failing on the test suite (Node 20/22/24). Mind taking a look? |
|
ok,i will do that |
|
No confident findings in the specified diff. The earlier [P1 blocking] Missing real-CLI verification record finding is resolved: the PR description documents a build followed by representative No PR code was run, built, or installed. |
|
@legend91019 CI is failing on the test suite (Node 20/22/24). Mind taking a look? |
|
yeah,i am testing |
|
No confident findings in the specified diff. The earlier [P1 blocking] Missing real-CLI verification record finding is resolved: the PR description documents a build followed by representative Only the specified diff was read. No PR code was run, built, or installed. |
|
@SaulMoro I think the pr is ready, anyway, the thing need you to talk with @jeff-r2026 . |
SaulMoro
left a comment
There was a problem hiding this comment.
Not approving yet.
Merge gate. On #1115, @jeff-r2026 asked us not to change the replace-whole behavior, and this PR changes it. I still prefer the tool-by-tool merge, but it needs Jeff's go-ahead before merge.
[P2 non-blocking] A trae-only override now fails to parse. With toolPaths: { trae: { mcpProject: .trae/team-mcp.json } }, the merged default trae-cn.mcpProject: .trae/mcp.json trips the #904 refine. loadTeamConfig then returns null for the whole file. Reproduced with the built CLI on bc91228 (teamai init prints ✖ Invalid teamai.yaml: toolPaths.trae-cn.mcpProject: ...). The same fixture parses on main. Suggested fix: when the team names only one of trae or trae-cn, carry its mcp and mcpProject over to the other before validation.
Summary
Extracts
DEFAULT_TOOL_PATHSinto an exported constant and updatesTeamaiConfigBaseSchema.toolPathsto merge user-providedtoolPathstool-by-tool over defaults, preventing partial tool definitions from discarding default configurations for all other built-in tools (such as Claude Code, Cursor, Codex, and CodeBuddy).Type of Change
Test Plan
npx tsc --noEmitpassesnpm run lintpasses (oxlint 0 errors, 0 warnings)npx vitest runpasses for affected test suites (types.test.ts,tool-roots.test.ts)src/__tests__/types.test.tsverifyingDEFAULT_TOOL_PATHSexport and default parse equivalencesrc/__tests__/types.test.tsverifying tool-by-tool merging preserves unmentioned built-in toolssrc/__tests__/types.test.tsverifying third-party custom agents can be declared alongside default toolssrc/__tests__/tool-roots.test.tsverifying sparse declarations override only the named tool while retaining default paths for other toolsnpm run build->node dist/index.js):src/__tests__/e2e/toolpaths-sparse-merge-1185.test.tsexercising compiled CLI (dist/index.js) against a local Git bare remote fixture with sparsetoolPathsdefinition (zcode: { skills: '.zcode/skills', rules: '.zcode/rules' }).init --provider git --agent claude,zcodeandpull):npm run test:e2e -- toolpaths-sparse-merge-1185.test.tspasses:Related Issues
Fixes #1185
Notes for Reviewers
src/types.ts,toolPathspreviously relied on.default({...})on the whole record. Supplying anytoolPathsinteamai.yamlbypassed.default(), wiping out all unmentioned tools.DEFAULT_TOOL_PATHSand used.default({}).transform((custom) => ({ ...DEFAULT_TOOL_PATHS, ...custom }))so that:mcpProject: .mcp.json).