Remove DEXTO_DEV_MODE and add DEXTO_HOME_DIR path override#633
Open
rahulkarajgikar wants to merge 10 commits intotruffle-ai:mainfrom
Open
Remove DEXTO_DEV_MODE and add DEXTO_HOME_DIR path override#633rahulkarajgikar wants to merge 10 commits intotruffle-ai:mainfrom
rahulkarajgikar wants to merge 10 commits intotruffle-ai:mainfrom
Conversation
|
@rahulkarajgikar is attempting to deploy a commit to the Shaunak's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR removes DEXTO_DEV_MODE, adds a DEXTO_HOME_DIR override, centralizes path resolution via getDextoPath/getDextoGlobalPath/getDextoEnvPath, replaces homedir() usages with those helpers, changes source-context handling to always use source root, and updates imports, tests, docs, and scripts accordingly. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant AgentMgmt as AgentManagement (path utils)
participant FS as Filesystem/Prefs
User->>CLI: start CLI / script (no --dev)
CLI->>AgentMgmt: getExecutionContext / getDextoGlobalPath(startPath)
AgentMgmt->>FS: resolve path (DEXTO_HOME_DIR? sourceRoot? global ~/.dexto)
FS-->>AgentMgmt: path result
AgentMgmt-->>CLI: return resolved paths (.env, models, skills, logs)
CLI->>FS: read preferences/repo configs using returned paths
FS-->>CLI: preferences / repo config
CLI->>User: proceed with resolved defaults (preferences prioritized)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 7
🧹 Nitpick comments (3)
packages/cli/src/utils/env.test.ts (1)
28-46: KeepgetDextoGlobalPathmock context detection aligned with thegetExecutionContextmock.This block duplicates context inference with narrower rules, which can drift and mask context-specific regressions in tests.
♻️ Suggested refactor
- const context = - fs.existsSync(path.join(startPath, 'package.json')) && - JSON.parse(fs.readFileSync(path.join(startPath, 'package.json'), 'utf-8')) - .name === 'dexto-monorepo' - ? 'dexto-source' - : 'global-cli'; + const pkgPath = path.join(startPath, 'package.json'); + let context: 'dexto-source' | 'dexto-project' | 'global-cli' = 'global-cli'; + if (fs.existsSync(pkgPath)) { + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8')); + if (pkg.name === 'dexto-monorepo' || pkg.name === 'dexto') { + context = 'dexto-source'; + } else if (pkg.dependencies?.dexto || pkg.devDependencies?.dexto) { + context = 'dexto-project'; + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/utils/env.test.ts` around lines 28 - 46, The getDextoGlobalPath mock duplicates the logic for detecting execution context and can drift from the getExecutionContext mock; modify getDextoGlobalPath to reuse or delegate to the same getExecutionContext mock (or call the exported getExecutionContext function) to determine whether context is 'dexto-source' or 'global-cli' rather than re-implementing the package.json check, then compute homeRoot and basePath from that shared result so both mocks stay aligned (refer to the getDextoGlobalPath and getExecutionContext symbols to locate and change the logic).packages/agent-management/src/plugins/discover-skills.ts (1)
99-101: Deduplicate skill directory paths before scanning/returning search paths.When resolved global skills path equals project
.dexto/skills, this can scan/list the same directory twice.♻️ Proposed dedupe refactor
export function discoverStandaloneSkills(projectPath?: string): DiscoveredSkill[] { const skills: DiscoveredSkill[] = []; const seenNames = new Set<string>(); const homeDir = process.env.HOME || process.env.USERPROFILE || ''; const cwd = projectPath || process.cwd(); const dextoSkillsDir = getDextoGlobalPath('skills', undefined, cwd); + const projectDextoSkillsDir = path.join(cwd, '.dexto', 'skills'); @@ - scanSkillsDir(path.join(cwd, '.dexto', 'skills'), 'project'); + scanSkillsDir(projectDextoSkillsDir, 'project'); @@ - scanSkillsDir(dextoSkillsDir, 'user'); + if (dextoSkillsDir !== projectDextoSkillsDir) { + scanSkillsDir(dextoSkillsDir, 'user'); + } return skills; } export function getSkillSearchPaths(): string[] { @@ - return [ + return Array.from( + new Set([ path.join(cwd, '.agents', 'skills'), path.join(cwd, '.dexto', 'skills'), homeDir ? path.join(homeDir, '.agents', 'skills') : '', dextoSkillsDir, - ].filter(Boolean); + ].filter(Boolean)) + ); }Also applies to: 109-109, 125-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-management/src/plugins/discover-skills.ts` around lines 99 - 101, scanSkillsDir is being called on potentially identical directories (e.g. project '.dexto/skills' and resolved global skills) which causes duplicate scans and returned search paths; change the code around the scanSkillsDir invocations (references: scanSkillsDir, cwd, '.agents/skills', '.dexto/skills', resolved global skills path) to deduplicate paths before scanning and before returning the search paths — e.g., gather candidate paths into a collection, normalize them, remove duplicates (Set or equality check), then iterate to call scanSkillsDir and build the final returned list; apply the same dedupe logic to the other spots noted (the calls around the same block at the other occurrences).packages/agent-management/src/utils/path.ts (1)
1-3: Consider collapsing this shim onto the core path utilities to avoid drift.Given this module is a temporary duplicate, keeping two path resolvers in sync across packages is a long-term maintenance hazard.
Based on learnings: Applies to **/*.{ts,tsx} : Use context-aware path resolution utilities from packages/core/src/utils/path.ts for path resolution that differs by execution context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-management/src/utils/path.ts` around lines 1 - 3, This shim duplicates the core path resolver and should be collapsed: remove the local duplicate implementations in this module and instead import and re-export the context-aware path utilities from the core package (use the same exported API this file currently exposes so callers remain unchanged), or directly delegate functions in this module to the core utilities; ensure TypeScript types/signatures match the original exports and update any local references to call the core implementation so the package uses the single source-of-truth path resolver.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@DEVELOPMENT.md`:
- Around line 214-219: Replace the misleading phrase "Setup checks skipped for
maintainer workflows" in the sentence under "When running in this repository
(`dexto-source` context)" with wording that reflects source-context behavior
(e.g., "Setup checks skipped in source context" or "Setup checks skipped when
running in repository source context"); update the line so the list item reads
consistently with the surrounding bullets about repository-local defaults and
does not reference maintainer-only semantics.
In `@packages/agent-management/src/models/path-resolver.ts`:
- Around line 14-15: Update the function doc for getDextoGlobalPath to state
that returned path is context-dependent: in normal contexts it maps to
<dexto-home>/models but in the "dexto-source" context it resolves to the
repository-local .dexto/models; mention both behaviors and the context switch so
callers understand when they will receive a global home path vs a repo-local
.dexto path and reference getDextoGlobalPath('models') and the "dexto-source"
context by name in the comment.
In `@packages/agent-management/src/utils/path.ts`:
- Around line 36-39: Update the doc comments to reflect the actual path shape
used when DEXTO_HOME_DIR is set: indicate that the implementation resolves paths
as <DEXTO_HOME_DIR>/<type> (not just <DEXTO_HOME_DIR>), and apply this
clarification to the context-aware sections for both resolvers in this file (the
source and project resolvers that read DEXTO_HOME_DIR). Ensure the comment lines
that currently say "Use <DEXTO_HOME_DIR>" are changed to "Use
<DEXTO_HOME_DIR>/<type>" (and similarly update the other instance around lines
89-91) so the documentation matches the implementation.
In `@packages/cli/CLI.md`:
- Around line 121-123: Update the Architecture section to reflect that the CLI
uses multiple local React state hooks rather than a centralized reducer: replace
the bullet "State Management - Centralized reducer pattern for predictable
state" with a concise line like "State Management - localized React useState
hooks (no centralized reducer)", and mention that the implementation in the CLI
component (referenced as ink-cli) intentionally uses multiple useState hooks;
ensure any other references to a reducer or centralized state (e.g., in "Custom
Hooks" or "Services" descriptions) are removed or reworded to avoid implying a
reducer-based pattern.
In `@packages/cli/src/cli/utils/setup-utils.ts`:
- Around line 117-121: Update the stale comment block in setup-utils.ts to
reflect the new "source skip" behavior: change the bullet list so "Source
context" indicates setup is skipped and preferences are not required/validated
(mirror "Project context: Skip setup"), and adjust the "First-time user" and
"Has preferences" bullets to specify they only apply to global-cli contexts;
ensure the wording clearly distinguishes source/global-cli vs project contexts
so the comment matches the runtime behavior.
In `@packages/core/src/utils/path.ts`:
- Around line 48-49: Replace the plain `throw new Error('...')` usages with the
core typed error factory for this module: locate each throw that uses the
message "Not in dexto source context" (and the other similar throw sites at the
additional ranges) in this file (e.g., the throws inside the path utilities) and
replace them with a module-specific typed error from the core error factory
(e.g., call the module's error helper like
createPathError('NotInDextoSourceContext', { detail: 'Not in dexto source
context' }) or similar), ensuring you import the factory helper and use
consistent error codes/names rather than raw Error instances for all occurrences
referenced (lines ~48, ~56, ~66, ~93, ~196, ~204).
In `@packages/tui/src/containers/OverlayContainer.tsx`:
- Around line 2271-2274: The code currently falls back silently to
getDextoGlobalPath('commands') when findDextoSourceRoot() returns falsy, which
can write prompts to an unexpected global location; change this to fail-fast (or
log an explicit error and abort) and use the context-aware path resolution
utilities from packages/core/src/utils/path.ts instead of getDextoGlobalPath.
Replace the conditional that sets commandsDir (which references
findDextoSourceRoot() and getDextoGlobalPath) with a call to the appropriate
path util (from packages/core/src/utils/path.ts) that resolves a commands path
based on execution context, and if it cannot resolve a source root ensure the
code throws or returns an explicit error/abort rather than silently using the
global path.
---
Nitpick comments:
In `@packages/agent-management/src/plugins/discover-skills.ts`:
- Around line 99-101: scanSkillsDir is being called on potentially identical
directories (e.g. project '.dexto/skills' and resolved global skills) which
causes duplicate scans and returned search paths; change the code around the
scanSkillsDir invocations (references: scanSkillsDir, cwd, '.agents/skills',
'.dexto/skills', resolved global skills path) to deduplicate paths before
scanning and before returning the search paths — e.g., gather candidate paths
into a collection, normalize them, remove duplicates (Set or equality check),
then iterate to call scanSkillsDir and build the final returned list; apply the
same dedupe logic to the other spots noted (the calls around the same block at
the other occurrences).
In `@packages/agent-management/src/utils/path.ts`:
- Around line 1-3: This shim duplicates the core path resolver and should be
collapsed: remove the local duplicate implementations in this module and instead
import and re-export the context-aware path utilities from the core package (use
the same exported API this file currently exposes so callers remain unchanged),
or directly delegate functions in this module to the core utilities; ensure
TypeScript types/signatures match the original exports and update any local
references to call the core implementation so the package uses the single
source-of-truth path resolver.
In `@packages/cli/src/utils/env.test.ts`:
- Around line 28-46: The getDextoGlobalPath mock duplicates the logic for
detecting execution context and can drift from the getExecutionContext mock;
modify getDextoGlobalPath to reuse or delegate to the same getExecutionContext
mock (or call the exported getExecutionContext function) to determine whether
context is 'dexto-source' or 'global-cli' rather than re-implementing the
package.json check, then compute homeRoot and basePath from that shared result
so both mocks stay aligned (refer to the getDextoGlobalPath and
getExecutionContext symbols to locate and change the logic).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
.claude/commands/dev-server.mdDEVELOPMENT.mdpackage.jsonpackages/agent-management/src/config/config-enrichment.tspackages/agent-management/src/config/discover-prompts.test.tspackages/agent-management/src/config/discover-prompts.tspackages/agent-management/src/config/loader.tspackages/agent-management/src/models/path-resolver.tspackages/agent-management/src/plugins/discover-skills.test.tspackages/agent-management/src/plugins/discover-skills.tspackages/agent-management/src/plugins/marketplace/__tests__/registry.test.tspackages/agent-management/src/plugins/marketplace/registry.tspackages/agent-management/src/resolver.test.tspackages/agent-management/src/resolver.tspackages/agent-management/src/utils/dexto-auth.tspackages/agent-management/src/utils/path.test.tspackages/agent-management/src/utils/path.tspackages/analytics/src/state.tspackages/cli/CLI.mdpackages/cli/src/cli/auth/dexto-api-key.tspackages/cli/src/cli/auth/service.tspackages/cli/src/cli/mcp/oauth-store.tspackages/cli/src/cli/utils/api-key-setup.tspackages/cli/src/cli/utils/package-mgmt.tspackages/cli/src/cli/utils/setup-utils.test.tspackages/cli/src/cli/utils/setup-utils.tspackages/cli/src/index-main.tspackages/cli/src/utils/env.test.tspackages/cli/src/utils/env.tspackages/core/src/llm/providers/local/ai-sdk-adapter.tspackages/core/src/utils/path.test.tspackages/core/src/utils/path.tspackages/image-logger-agent/src/hooks/request-logger.tspackages/tui/src/containers/OverlayContainer.tsxpackages/tui/src/utils/soundNotification.tsscripts/dev-server.ts
💤 Files with no reviewable changes (2)
- scripts/dev-server.ts
- packages/cli/src/index-main.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/cli/CLI.md (2)
160-160: Consider adding default path for user convenience.For troubleshooting, mentioning the default location might help users locate their session files more quickly.
📝 Optional enhancement
-Sessions are stored in `.dexto/sessions/` under the resolved Dexto home +Sessions are stored in `.dexto/sessions/` under the resolved Dexto home (`~/.dexto` by default)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/CLI.md` at line 160, Update the Sessions documentation line that currently reads "Sessions are stored in `.dexto/sessions/` under the resolved Dexto home" to include the default resolved Dexto home path for convenience (e.g., expand to show the default like "~/.config/dexto/.dexto/sessions" or the platform-specific defaults), so users can immediately locate session files; keep the existing phrasing but append the default path example and note that it may vary by OS.
171-176: Consider clarifying that DEXTO_HOME_DIR is the authoritative override.The PR emphasizes DEXTO_HOME_DIR as the "highest-precedence override," but the comment only mentions "custom Dexto home for local testing." Adding a brief note about it overriding all other path resolution would align with the PR's design intent.
📝 Optional clarification
-# Use custom Dexto home for local testing +# Override Dexto home directory (highest precedence) export DEXTO_HOME_DIR=/tmp/dexto dexto --mode cliOr keep it user-friendly but mention the override behavior:
-# Use custom Dexto home for local testing +# Use custom Dexto home for local testing (overrides default ~/.dexto) export DEXTO_HOME_DIR=/tmp/dexto dexto --mode cli🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/CLI.md` around lines 171 - 176, Update the "Home Directory Override" section and the example to state that the environment variable DEXTO_HOME_DIR is the authoritative, highest-precedence override for Dexto's home directory (it supersedes any other path resolution or config). In the "Home Directory Override" header and the code snippet area, add a short sentence clarifying that setting DEXTO_HOME_DIR will override all other home directory settings so users understand its precedence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent-management/src/utils/path.ts`:
- Around line 89-96: Update the JSDoc return description for the function
documented by the comment with params (type, filename, startPath) to reflect
that it can return three kinds of absolute paths: the DEXTO_HOME_DIR/<type> when
DEXTO_HOME_DIR is set, the repo-local .dexto/<type> when running in source
context, or the global ~/.dexto/<type> otherwise; mention that filename is
appended when provided and that the returned value is an absolute path.
---
Nitpick comments:
In `@packages/cli/CLI.md`:
- Line 160: Update the Sessions documentation line that currently reads
"Sessions are stored in `.dexto/sessions/` under the resolved Dexto home" to
include the default resolved Dexto home path for convenience (e.g., expand to
show the default like "~/.config/dexto/.dexto/sessions" or the platform-specific
defaults), so users can immediately locate session files; keep the existing
phrasing but append the default path example and note that it may vary by OS.
- Around line 171-176: Update the "Home Directory Override" section and the
example to state that the environment variable DEXTO_HOME_DIR is the
authoritative, highest-precedence override for Dexto's home directory (it
supersedes any other path resolution or config). In the "Home Directory
Override" header and the code snippet area, add a short sentence clarifying that
setting DEXTO_HOME_DIR will override all other home directory settings so users
understand its precedence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35714ee6-6e5d-4399-ab86-354a34218c54
📒 Files selected for processing (6)
DEVELOPMENT.mdpackages/agent-management/src/models/path-resolver.tspackages/agent-management/src/utils/path.tspackages/cli/CLI.mdpackages/cli/src/cli/utils/setup-utils.tspackages/tui/src/containers/OverlayContainer.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/tui/src/containers/OverlayContainer.tsx
- packages/cli/src/cli/utils/setup-utils.ts
- DEVELOPMENT.md
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/agent-management/src/utils/path.ts (1)
53-71: Avoid repeated root discovery after context detection.
getExecutionContext(startPath)already performs directory-walk checks, then each branch repeats root discovery. Consider resolving roots/context once per function and reusing the result to reduce repeated filesystem traversal and simplify control flow.Also applies to: 105-107, 262-278
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-management/src/utils/path.ts` around lines 53 - 71, The switch uses getExecutionContext(startPath) but then calls findDextoSourceRoot/startPath and findDextoProjectRoot/startPath again — remove the duplicated walks by resolving the root once and reusing it: call getExecutionContext(startPath) and then immediately compute the appropriate root variable (e.g., sourceRoot via findDextoSourceRoot or projectRoot via findDextoProjectRoot) before the switch (or compute both and pick the correct one), throw if missing, and then set basePath = path.join(resolvedRoot, '.dexto', type); apply the same refactor to the other similar blocks that use findDextoSourceRoot/findDextoProjectRoot (the blocks referencing lines ~105-107 and ~262-278) so filesystem traversal happens only once per function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent-management/src/utils/path.ts`:
- Around line 235-236: Update the JSDoc/comment for the function
ensureDextoGlobalDirectory to reflect that it ensures the resolved storage
directory (which may be source-local, DEXTO_HOME_DIR, or a global directory)
rather than implying it always creates a global storage directory; locate the
comment above the ensureDextoGlobalDirectory symbol and reword it to mention
"resolved storage directory" or "resolved Dexto storage directory" and that it
may be global or source-local depending on configuration.
---
Nitpick comments:
In `@packages/agent-management/src/utils/path.ts`:
- Around line 53-71: The switch uses getExecutionContext(startPath) but then
calls findDextoSourceRoot/startPath and findDextoProjectRoot/startPath again —
remove the duplicated walks by resolving the root once and reusing it: call
getExecutionContext(startPath) and then immediately compute the appropriate root
variable (e.g., sourceRoot via findDextoSourceRoot or projectRoot via
findDextoProjectRoot) before the switch (or compute both and pick the correct
one), throw if missing, and then set basePath = path.join(resolvedRoot,
'.dexto', type); apply the same refactor to the other similar blocks that use
findDextoSourceRoot/findDextoProjectRoot (the blocks referencing lines ~105-107
and ~262-278) so filesystem traversal happens only once per function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8a1e21df-911e-40c7-8222-80005b7c52c9
📒 Files selected for processing (1)
packages/agent-management/src/utils/path.ts
Collaborator
Author
|
Smoke pass completed after Checks run:
Net: the new home-dir override, source-context defaults, bundled agent resolution, and config template expansion all behave correctly in the exercised flows. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DEXTO_DEV_MODEand--devbranchingcommands/and repo.dexto/paths)DEXTO_HOME_DIRas highest-precedence override for Dexto storage and env-loading paths~/.dextousages with shared path resolversValidation
bash scripts/quality-checks.shsync-openapi-docs:checkdue existing OpenAPI drift in this branchpnpm exec vitest run packages/agent-management/src/utils/path.test.ts packages/core/src/utils/path.test.ts packages/agent-management/src/plugins/discover-skills.test.ts packages/agent-management/src/plugins/marketplace/__tests__/registry.test.ts packages/agent-management/src/config/discover-prompts.test.ts packages/agent-management/src/resolver.test.ts packages/cli/src/cli/utils/setup-utils.test.ts packages/cli/src/utils/env.test.tsnode packages/cli/dist/index.js --helpnode packages/cli/dist/index.js auth statusDEXTO_HOME_DIR=/tmp/dexto-smoke node packages/cli/dist/index.js which coding-agentSummary by CodeRabbit
New Features
Bug Fixes & Improvements
Documentation