Fix/strip redundant plan prefix#25587
Draft
moisgobg wants to merge 19 commits intofix/nested-plan-files-v3from
Draft
Fix/strip redundant plan prefix#25587moisgobg wants to merge 19 commits intofix/nested-plan-files-v3from
moisgobg wants to merge 19 commits intofix/nested-plan-files-v3from
Conversation
…tion Introduces active extension context tracking in config to support dynamic switching of plan directories. Resolves circular dependency in storage by deferring plan directory creation until on-demand use, preventing ENOENT errors on non-existent paths.
Updates prompts and tool implementations (edit, write-file, enter/exit plan mode) to route through Config.getPlansDir() instead of Storage.getPlansDir(). This ensures the plan directory is lazily created exactly when these features attempt to use it, preventing ENOENT failures.
Extracts the extension context from slash commands based on their registered metadata and sets it as the active context in the Config before execution. This enables the backend to dynamically route plan directories based on the extension that owns the invoked command.
…lution This commit addresses two bugs identified during review: 1. Cleared the sticky `activeExtensionContext` when the standard `/plan` command is executed, ensuring subsequent prompts correctly target the default global plan directory. 2. Fixed a path resolution regression in `Storage.getPlansDir()` by constructing the fallback ENOENT path directly against the real project root. This prevents `isSubpath` validation failures and potential traversal vulnerabilities when the project root is a symlink.
This fixes a bug where the active extension context would remain sticky when a user switched from an extension command to a standard non-plan command, or to an extension without a plan directory. The context is now correctly reset to undefined when an extension command without a plan directory is executed, preventing subsequent plan mode invocations from incorrectly targeting the previous extension's folder.
Adds caching to getPlansDir to avoid redundant synchronous disk I/O and repeated workspace context registrations.
This addresses a potential TOCTOU vulnerability and edge case identified during review. The redundant `fs.existsSync` check in `getPlansDir` has been removed, allowing `fs.mkdirSync(..., { recursive: true })` to safely handle directory idempotency.
By relying directly on `mkdirSync`, we ensure that if a non-directory file already exists at the target path, the system will correctly throw an `EEXIST` error rather than silently treating the file as a directory and crashing later during workspace registration.
…ctive plan mode Address review bot feedback to prevent EACCES errors during startup by restricting filesystem mutation and workspace context registration to when the user is actively in an interactive plan session. Non-plan tool registration now uses the safely resolved path without attempting to create the directory.
…ules Addressed reviewer feedback by replacing process.stderr.write with debugLogger.error in config.ts and using isNodeError in storage.ts to adhere to codebase standards.
Align the 'foo' test prompt with existing project conventions while ensuring the model has the 'informal agreement' signal required to proceed to formal approval and implementation.
…sing Moves the getErrorMessage import to the top of config.ts to avoid circularity-induced TS2304 in some environments. Also fixes a TypeError in sandbox_command.js when modern object-style sandbox settings are used in settings.json.
Introduces a retry mechanism for fs.promises.rename when saving the project registry. This resolves a known concurrency issue on Windows CI runners where multiple processes spinning up simultaneously during E2E tests cause file lock contention.
Adds a hard limit to the while(true) loop in claimNewSlug to prevent tests from hanging indefinitely when fs.existsSync is mocked improperly. Refines error typing in the rename retry block to strictly use isNodeError.
Resolves conflict in packages/core/src/tools/enter-plan-mode.test.ts by removing an assertion for directory creation, which has been centralized in config.ts in this branch.
This fixes a bug where path.basename was incorrectly stripping directory structures from plan files (e.g., trying to write to plans/nested/file.md would incorrectly write to plans/file.md). By using path.resolve and verifying with isSubpath, nested files are now handled securely and correctly.
…consistently This fixes unit test failures by centralizing path resolution logic.
This fixes test failures introduced during conflict resolution by strictly calling storage.getPlansDir as per the new config architecture.
This fix updates `resolveAndValidatePlanPath` to detect and strip redundant prefixes from file paths when the agent provides a path that begins with the basename of the plans directory (e.g., stripping `conductor/` when writing to `conductor/product.md` inside the `conductor` plan directory). This prevents the creation of nested directories (e.g., `conductor/conductor/product.md`) while maintaining path compatibility between Plan Mode and Execution Mode.
…an_mode - **exit_plan_mode.ts**: Fixed an issue where `path.basename` incorrectly stripped nested directories (e.g., `tracks/123/index.md` to `index.md`) by adopting `resolveAndValidatePlanPath`. - **UI Components**: Updated `ExitPlanModeDialog.tsx` and `planCommand.ts` to use `config.getPlansDir()` instead of `config.storage.getPlansDir()`. This ensures the UI respects active extension plan directories (like Conductor) rather than falling back to the default temporary directory. - **write-file.ts**: Updated to use `config.getPlansDir()` for consistency.
mahimashanware
force-pushed
the
fix/nested-plan-files-v3
branch
from
261e57c to
59cc707
Compare
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
2 participants
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.
Commit 1: Fixing Directory Nesting
conductor/product.md). Because Plan Mode is already rooted in that folder, this caused files to be saved in a
redundant nested folder (conductor/conductor/product.md).
directory name (like conductor/ or ./conductor/) before final resolution.
Commit 2: Fixing Plan Mode Approval & UI
tracks/123/index.md into just index.md). Simultaneously, the UI was failing to display plans because it was
looking in the project's temporary folder instead of the Conductor extension's plan folder.
plan directory through the extension-aware config service rather than directly from low-level storage.