chore(platform): seed personal dev config dir from examples on first run#1731
chore(platform): seed personal dev config dir from examples on first run#1731Israeltheminer wants to merge 3 commits into
Conversation
The dev orchestrator was defaulting `TALE_CONFIG_DIR` to the in-repo `examples/` directory. Every provider edit, model save, secret rotation, or `fetchConfiguredProviderModels` call writes back through that path, which means routine UI clicks show up in `git status` and risk riding along on unrelated PRs (see #1729 where two `examples/providers/*.json` edits had to be reverted from a provider-drawer feature PR). Switch the default to a personal, gitignored location: `~/.tale/dev-config/`. On first run the directory is seeded from `examples/` (one-time, atomic, `.DS_Store` filtered). On subsequent runs the seed is skipped, so local edits persist across sessions and `git pull` updates to `examples/` no longer clobber per-developer state. `TALE_CONFIG_DIR` in `.env.local` still wins, so anyone pointing at a customer config or a shared checkout keeps that behaviour. Also logs the active `TALE_CONFIG_DIR` on startup so the override is discoverable without grepping the script.
📝 WalkthroughWalkthroughThe pull request modifies Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/platform/scripts/dev.ts`:
- Around line 67-92: The seedDevConfigDir function currently copies directly
into target with cpSync so a partial copy leaves an incomplete
~/.tale/dev-config on failure; change the flow to copy into a temp directory
(e.g., join(homedir(), '.tale', `dev-config.tmp.${Date.now()}`)) under the same
parent, using cpSync with the same options, then atomically rename/replace temp
→ target using renameSync; ensure you create the parent dir with mkdirSync
before copying, catch and cleanup the temp dir on errors, and keep the
existsSync(target) short-circuit unchanged so subsequent runs see the final
atomic rename.
- Line 86: The filter arrow function currently uses src.endsWith('/.DS_Store')
which is not cross-platform; update the import from 'path' to include basename
and change the predicate in the filter (the (src) => ...) to check
path.basename(src) !== '.DS_Store' so it works on Windows and POSIX paths;
locate the filter definition that contains "(src) =>
!src.endsWith('/.DS_Store')" and the top-of-file path import to add basename.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7f045066-b03c-479d-9d9b-01eab47d27a4
📒 Files selected for processing (1)
services/platform/scripts/dev.ts
| function seedDevConfigDir(): string { | ||
| const target = join(homedir(), '.tale', 'dev-config'); | ||
| if (existsSync(target)) return target; | ||
|
|
||
| const source = join(repoRoot, 'examples'); | ||
| if (!existsSync(source)) { | ||
| // No examples to seed from; just create an empty dir so the convex | ||
| // file_utils don't blow up on the first read. | ||
| mkdirSync(target, { recursive: true }); | ||
| console.log( | ||
| `[dev] 📁 Created empty dev config dir (no examples/ to seed from): ${target}`, | ||
| ); | ||
| return target; | ||
| } | ||
|
|
||
| mkdirSync(join(homedir(), '.tale'), { recursive: true }); | ||
| cpSync(source, target, { | ||
| recursive: true, | ||
| // Skip macOS metadata — would pollute the seeded tree. | ||
| filter: (src) => !src.endsWith('/.DS_Store'), | ||
| }); | ||
| console.log( | ||
| `[dev] 📁 Seeded dev config from ${source} → ${target} (one-time; your edits stay local)`, | ||
| ); | ||
| return target; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Seeding is not truly atomic as described in PR objectives.
If cpSync fails partway through (e.g., disk full, permission error), the target directory will contain partial content. On the next run, existsSync(target) returns true (line 69), so the seed is skipped, leaving the developer with an incomplete config directory.
For a dev script, this may be acceptable—developers can manually delete ~/.tale/dev-config and re-run. If true atomicity is required, consider copying to a temporary directory first, then renaming it to the final location in a single operation.
💡 Approach for atomic seeding
const temp = join(homedir(), '.tale', `dev-config.tmp.${Date.now()}`);
mkdirSync(join(homedir(), '.tale'), { recursive: true });
cpSync(source, temp, {
recursive: true,
filter: (src) => basename(src) !== '.DS_Store',
});
// Atomic rename (fails if target exists)
renameSync(temp, target);Requires importing renameSync from node:fs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/scripts/dev.ts` around lines 67 - 92, The seedDevConfigDir
function currently copies directly into target with cpSync so a partial copy
leaves an incomplete ~/.tale/dev-config on failure; change the flow to copy into
a temp directory (e.g., join(homedir(), '.tale',
`dev-config.tmp.${Date.now()}`)) under the same parent, using cpSync with the
same options, then atomically rename/replace temp → target using renameSync;
ensure you create the parent dir with mkdirSync before copying, catch and
cleanup the temp dir on errors, and keep the existsSync(target) short-circuit
unchanged so subsequent runs see the final atomic rename.
| cpSync(source, target, { | ||
| recursive: true, | ||
| // Skip macOS metadata — would pollute the seeded tree. | ||
| filter: (src) => !src.endsWith('/.DS_Store'), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use cross-platform path comparison for .DS_Store filtering.
The filter !src.endsWith('/.DS_Store') assumes forward-slash path separators, which may not work correctly on Windows where paths use backslashes. Using path.basename() is more explicit and cross-platform.
🔧 Suggested cross-platform filter
First, add basename to the path import on line 29:
-import { join } from 'node:path';
+import { basename, join } from 'node:path';Then update the filter:
cpSync(source, target, {
recursive: true,
- // Skip macOS metadata — would pollute the seeded tree.
- filter: (src) => !src.endsWith('/.DS_Store'),
+ // Skip macOS metadata — would pollute the seeded tree.
+ filter: (src) => basename(src) !== '.DS_Store',
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/scripts/dev.ts` at line 86, The filter arrow function
currently uses src.endsWith('/.DS_Store') which is not cross-platform; update
the import from 'path' to include basename and change the predicate in the
filter (the (src) => ...) to check path.basename(src) !== '.DS_Store' so it
works on Windows and POSIX paths; locate the filter definition that contains
"(src) => !src.endsWith('/.DS_Store')" and the top-of-file path import to add
basename.
CodeRabbit feedback on #1731: - Copy into a sibling temp dir, then renameSync into place. Prevents a crashed/interrupted seed from leaving a half-populated dev-config/ that the existsSync short-circuit would then treat as already seeded on the next run. Cleanup on error so failed runs don't leave orphan temp dirs. - Use path.basename for the .DS_Store filter instead of endsWith on a POSIX slash, so the filter still works on Windows (which uses backslashes).
1 participant
Summary
TALE_CONFIG_DIRto the in-repoexamples/directory. Every provider edit, model save, secret rotation, orfetchConfiguredProviderModelscall writes back through that path, so routine UI clicks show up ingit statusand can ride along on unrelated PRs (see feat(platform): provider detail drawer + api-key suffix masking #1729 where twoexamples/providers/*.jsonedits had to be reverted from a provider-drawer feature PR).~/.tale/dev-config/. On first run the directory is seeded fromexamples/(one-time, atomic,.DS_Storefiltered). On subsequent runs the seed is skipped, so local edits persist across sessions andgit pullupdates toexamples/no longer clobber per-developer state.TALE_CONFIG_DIRin.env.localstill wins, so anyone pointing at a customer config or a shared checkout keeps that behaviour.TALE_CONFIG_DIRon startup so the override is discoverable without grepping the script.Test plan
~/.tale/dev-config/):bun run devseeds and logs📁 Seeded dev config from … → ~/.tale/dev-config/ (one-time …)followed by📂 TALE_CONFIG_DIR=….bun run dev: no seed message, just📂 TALE_CONFIG_DIR=….~/.tale/dev-config/providers/*.jsonupdates;examples/providers/*.jsonstays clean.TALE_CONFIG_DIR=/tmp/custom-configin.env.local: startup log reflects the override, seed is skipped.Summary by CodeRabbit