fix(create-agents): sync template deps to CLI version after clone#2289
fix(create-agents): sync template deps to CLI version after clone#2289miles-kt-inkeep merged 12 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: e9436b0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
PR Review Summary
(2) Total Issues | Risk: Low
🟠⚠️ Major (1) 🟠⚠️
Inline Comments:
- 🟠 Major:
utils.test.ts:32Missing test coverage forgetCliVersion()failure path
🟡 Minor (1) 🟡
Inline Comments:
- 🟡 Minor:
utils.ts:58Nested workspace packages not synced — root-only sync may leaveapps/*with stale versions
💭 Consider (0) 💭
None.
🧹 While You're Here (0) 🧹
None.
💡 APPROVE WITH SUGGESTIONS
Summary: This is a well-implemented fix for a real DX pain point. The core logic is solid — version resolution via import.meta.url, graceful degradation on errors, and proper JSON formatting. The test coverage is good but has one gap worth addressing: the getCliVersion() failure branch isn't exercised. The nested workspace sync question is worth clarifying but may be intentional given pnpm's hoisting behavior.
Discarded (4)
| Location | Issue | Reason Discarded |
|---|---|---|
utils.ts:47-55 |
Silent failure in getCliVersion() could mask debugging issues |
Intentional design for graceful degradation — acceptable UX for scaffolding CLI |
utils.ts:61 |
No test for fs.readJson throwing on malformed JSON |
Edge case that would surface during template development; current throw behavior is acceptable |
utils.ts:75 |
Unnecessary write when no @inkeep/* deps exist |
Minor inefficiency, not a bug — file content identical |
| Template lockfile | Stale pnpm-lock.yaml after dep sync |
Ordering is correct: sync happens before install which regenerates lockfile |
Reviewers (4)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
3 | 0 | 0 | 0 | 0 | 0 | 3 |
pr-review-tests |
2 | 0 | 0 | 0 | 1 | 0 | 1 |
pr-review-devops |
5 | 0 | 0 | 0 | 0 | 0 | 5 |
pr-review-product |
3 | 0 | 0 | 0 | 1 | 0 | 2 |
| Total | 13 | 0 | 0 | 0 | 2 | 0 | 11 |
Review Response🟠 Missing test for
|
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Delta Review
This re-review covers changes since the prior automated review (0d60b0e24 → eaf46e621).
Prior Major issue — resolved ✅
The missing test for getCliVersion() failure path has been addressed. The new test at utils.test.ts:826-840:
- Overrides
readFileSyncto throwENOENT - Ensures
pathExistsreturnstrue(bypasses first guard) - Provides mock package.json data
- Verifies
writeJsonwas NOT called — confirming the early return atutils.ts:63was hit
The test correctly exercises the graceful degradation path: when CLI version detection fails, syncing is silently skipped.
Prior Minor issue — declined (acknowledged)
The nested workspace packages not synced issue was declined by the author with valid justification: pnpm workspaces use a single lockfile, so nested @inkeep/* deps resolve to the same version as root deps. No action needed.
✅ APPROVE
Summary: Prior feedback addressed. The test coverage gap has been filled with a well-structured test that correctly validates the failure path. The implementation is solid and ready to merge. 🎉
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| N/A — delta-only re-review | — | — | — | — | — | — | — |
Note: Subagent reviewers were not dispatched for this delta review. The change is a 16-line test addition that directly addresses prior feedback — full subagent review not warranted.
Ito Test Report ✅20 test cases ran. 20 passed. All tests for the ✅ Passed (20)
📋 View Recording |
Derive from monorepo .env.example with all vars needed by runSetup(): DB URLs, API URLs, SpiceDB, bypass secret, MCP OAuth, OTEL/SigNoz/Nango with local defaults. Add DEFAULT_PROJECT_ID and AZURE_API_KEY. Omit GitHub App, Slack, PostHog, and feature flag sections. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…cret generation Rename generateJwtKeys() → generateSecrets() and extend to handle 5 secrets: - JWT key pair (existing, RSA 2048-bit) - INKEEP_AGENTS_JWT_SIGNING_SECRET (hex, 32 bytes) - BETTER_AUTH_SECRET (hex, 32 bytes, replaces placeholder) - INKEEP_AGENTS_MANAGE_UI_PASSWORD (base64url, 6 bytes, replaces placeholder) Conservative: only replaces exact placeholder strings or empty values. Preserves user-customized values. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…ct CLI values Replace hardcoded template string with copy+inject pattern: - Copy .env.example to .env (comprehensive template from US-001) - Inject CLI-prompted values (API keys, DEFAULT_PROJECT_ID) - Remove all inline secret generation (deferred to generateSecrets) - .env now contains INKEEP_AGENTS_MANAGE_API_BYPASS_SECRET - Uses localhost URLs instead of 127.0.0.1 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Wrap fs.readFile('.env.example') in try-catch with a clear error message
when the template file is missing or corrupted after degit clone.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Change OTEL_EXPORTER_OTLP_TRACES_ENDPOINT from active localhost value to empty string in template .env.example. Empty value causes the template's instrumentation guard to skip OTEL initialization, avoiding ~8s request timeouts when no collector is running. setup-dev:optional replaces the value in-place (no duplicate lines). - Add commented OTEL_EXPORTER_OTLP_TRACES_HEADERS with note that it's only needed for SigNoz Cloud. - Prepend turbo build to test:e2e scripts so standalone dashboard artifacts exist before the E2E test runs. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Update mockEnvExample to use empty OTEL_EXPORTER_OTLP_TRACES_ENDPOINT matching the actual create-agents-template/.env.example value. Co-Authored-By: Claude Opus 4.6 <[email protected]>
After cloning the template via degit, reads the CLI's own package.json
version and updates all @inkeep/* deps in the template's package.json
to ^{cliVersion}. This prevents version mismatch when the template
hasn't been updated to match the latest CLI release.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Exercises the early-return branch when CLI version detection fails (readFileSync throws). Verifies syncTemplateDependencies gracefully skips without writing to disk. Co-Authored-By: Claude Opus 4.6 <[email protected]>
29ef11d to
47804c0
Compare
There was a problem hiding this comment.
PR Review Summary
(1) Total Issues | Risk: Low
Delta Review
This re-review covers changes since the prior automated review (f21a96130 → 47804c01f).
New changes in 47804c01f:
- Implemented recursive
findPackageJsonFiles()function to sync nested package.json files - Added
@inkeep/agents-uiexclusion from version syncing - Uses
Promise.all()for parallel processing of multiple package.json files - Added 2 new test cases for nested syncing and directory skipping
Prior issues — resolved ✅
Both prior review issues have been addressed:
- ✅ Missing test for
getCliVersion()failure path — Test added inf21a96130 - ✅ Nested workspace packages not synced — Implemented in
47804c01f(recursive sync now covers all nested package.json files)
🟡 Minor (1) 🟡
Inline Comments:
- 🟡 Minor:
utils.test.ts:921Missing test for@inkeep/agents-uiexclusion
💭 Consider (0) 💭
None.
🧹 While You're Here (0) 🧹
None.
💡 APPROVE WITH SUGGESTIONS
Summary: The recursive nested package.json syncing is a solid improvement that addresses the prior review feedback. The implementation is clean with proper directory filtering (skipping node_modules and dot directories) and parallel processing via Promise.all(). The one gap is a missing test for the @inkeep/agents-ui exclusion — this is a minor issue since the exclusion logic is intentional but untested. Overall, this is ready to merge with an optional test addition. 🚀
Discarded (1)
| Location | Issue | Reason Discarded |
|---|---|---|
utils.test.ts |
No test for fs.readJson failure during sync |
Edge case with low criticality (3/10 per reviewer). Fail-fast behavior is reasonable for scaffolding CLI — throwing on malformed JSON is acceptable default behavior. |
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-product |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-tests |
2 | 0 | 0 | 0 | 1 | 0 | 1 |
| Total | 2 | 0 | 0 | 0 | 1 | 0 | 1 |
| withFileTypes: true, | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🟡 Minor: Missing test for @inkeep/agents-ui exclusion
Issue: The implementation at utils.ts:71 explicitly excludes @inkeep/agents-ui from version syncing (name !== '@inkeep/agents-ui'), but no test verifies this behavior.
Why: If the exclusion condition is accidentally removed or inverted in a future refactor, @inkeep/agents-ui would be synced to the CLI version. Since agents-ui has its own versioning cadence separate from the core packages, this could cause version mismatches.
Fix: Add a test case to verify @inkeep/agents-ui is not updated:
it('should NOT update @inkeep/agents-ui dependency', async () => {
const mockPkg = {
name: 'test-project',
dependencies: {
'@inkeep/agents-core': '^0.50.3',
'@inkeep/agents-ui': '^0.15.12',
},
};
setupFlatTemplate(mockPkg);
await syncTemplateDependencies('/test/path');
expect(fs.writeJson).toHaveBeenCalledWith(
'/test/path/package.json',
expect.objectContaining({
dependencies: {
'@inkeep/agents-core': '^1.2.3',
'@inkeep/agents-ui': '^0.15.12', // unchanged
},
}),
{ spaces: 2 }
);
});Refs:
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Delta Review
This re-review covers changes since the prior automated review (47804c01f → e9436b024).
Changes in e9436b024:
- Added
fs.readdirmock to test setup functions (lines 117, 673) — required for the recursivefindPackageJsonFiles()implementation - Added new test case
'should not update excluded packages like @inkeep/agents-ui'(lines 830-854)
Prior Minor issue — resolved ✅
The missing test for @inkeep/agents-ui exclusion has been addressed. The new test:
- Sets up a mock package with
@inkeep/agents-core,@inkeep/agents-ui, and@inkeep/agents-sdkall at^0.50.3 - Calls
syncTemplateDependencies - Verifies that
agents-coreandagents-sdkare updated to^1.2.3, butagents-uiremains unchanged at^0.50.3
This directly validates the exclusion logic at utils.ts:71.
✅ APPROVE
Summary: All prior feedback addressed! The test coverage is now complete — the @inkeep/agents-ui exclusion path is properly validated. This PR is ready to merge. 🎉
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| N/A — delta-only re-review | — | — | — | — | — | — | — |
Note: Subagent reviewers were not dispatched for this delta review. The change is a 28-line test addition that directly addresses prior feedback — full subagent review not warranted.
Ito Test Report ✅20 test cases ran. 20 passed. This test run verified the new dependency version sync feature in ✅ Passed (20)
📋 View Recording |
Problem
When a user runs
npx @inkeep/create-agents@latest, the CLI clonescreate-agents-templatefrom GitHub HEAD viadegit. The template repo has its ownpackage.jsonwith@inkeep/*dependencies pinned at whatever version was last manually committed — and that version always lags behind the CLI release.Example: CLI publishes v0.52.0, but the template still has
@inkeep/agents-core: "^0.50.3". Afterpnpm install, the user ends up with mismatched package versions: the CLI scaffolded code that expects v0.52.0 APIs, but the installed packages are v0.50.x. This causes cryptic runtime errors that are impossible for a new user to diagnose.The root cause is that template dep versions and CLI versions are updated independently — there's no mechanism to keep them in sync at clone time. This PR fixes that.
Solution
After cloning the template and
chdir-ing into the project directory, the CLI now:package.jsonversion (the version the user is actually running)@inkeep/*entries in the template'spackage.jsonto^{cliVersion}pnpm installas before — now with matching versionsThis runs before env file generation and install, so the user gets correct deps from the start. If the CLI version can't be read (e.g., running from source), it silently skips the sync — no breaking change.
Stacked on #2212 — contains only the dep-sync logic, no overlap with env generation changes.
What changed
packages/create-agents/src/utils.ts(+35 lines)getCliVersion()— resolves the CLI's ownpackage.jsonviaimport.meta.url, returns theversionfield (or''on any failure)syncTemplateDependencies(templatePath)— reads the template'spackage.json, iteratesdependenciesanddevDependencies, updates any@inkeep/*entry to^{cliVersion}, writes the file backawait syncTemplateDependencies('.')added increateAgents()afterprocess.chdir(directoryPath), before env file generationpackages/create-agents/src/__tests__/utils.test.ts(+161 lines)node:fsmock (readFileSync→ returns{ version: '1.2.3' })node:urlmock (fileURLToPath→ returns/fake/dist/utils.js)@inkeep/*deps to^1.2.3package.jsondoesn't exist@inkeep/*depsdevDependencieskey@inkeep/*indevDependencies@inkeepdeps (react,next, etc.) untouched.changeset/meaningful-orange-kangaroo.md@inkeep/create-agentsWhat this does NOT touch
createEnvironmentFiles()— handled by Unify .env generation between quickstart CLI and contributor flows #2212createInkeepConfig()— handled by Unify .env generation between quickstart CLI and contributor flows #2212Test plan
tsc --noEmit— passesbiome lint --error-on-warnings— passesbiome check(format) — passessyncTemplateDependenciestests pass🤖 Generated with Claude Code