- Notifications
You must be signed in to change notification settings - Fork2.8k
Use workdir as root for everything instead of repo root#526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR changes the scripts to use the current working directory as the root for file operations instead of the repository root directory. This enables the scripts to work on subdirectories of a repository or non-git managed directories.
Key changes:
- Replace
REPO_ROOTvariable withWORKING_DIRthroughout all scripts - Set
WORKING_DIRto current working directory (pwd) instead of git repository root - Update all file path constructions to use working directory as base
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/powershell/update-agent-context.ps1 | Replace REPO_ROOT with WORKING_DIR for agent file paths and project name extraction |
| scripts/powershell/setup-plan.ps1 | Update template path to use working directory |
| scripts/powershell/create-new-feature.ps1 | Simplify to use working directory, remove complex repository root detection |
| scripts/powershell/common.ps1 | Add WORKING_DIR variable and update path functions |
| scripts/powershell/check-prerequisites.ps1 | Update output to use WORKING_DIR instead of REPO_ROOT |
| scripts/bash/update-agent-context.sh | Replace REPO_ROOT with WORKING_DIR for agent file paths |
| scripts/bash/setup-plan.sh | Update template path to use working directory |
| scripts/bash/create-new-feature.sh | Simplify to use working directory, remove repository root detection |
| scripts/bash/common.sh | Add WORKING_DIR variable and update path functions |
| scripts/bash/check-prerequisites.sh | Update output to use WORKING_DIR instead of REPO_ROOT |
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
fix for#516 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
| } | ||
| get_feature_dir() {echo"$1/specs/$2"; } | ||
| get_feature_dir() {echo"$WORKING_DIR/specs/$1"; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The function signature changed from accepting two parameters (repo_root and branch) to one parameter (branch), but the function body references$1 which now represents the branch parameter. However, the function is called elsewhere in the codebase expecting the old two-parameter signature, which could cause incorrect behavior.
Copilot uses AI. Check for mistakes.
scripts/powershell/common.ps1 Outdated
| # Set working directory | ||
| $script:WorkingDir= (Get-Location).Path | ||
| # Function removed - no longer needed since we use WorkingDir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[nitpick] This comment is unclear about which function was removed. Consider being more specific, such as '# Get-RepoRoot function removed - no longer needed since we use WorkingDir'.
| #Function removed - no longer needed since we use WorkingDir | |
| #Get-RepoRoot function removed - no longer needed since we use WorkingDir |
Copilot uses AI. Check for mistakes.
scripts/bash/common.sh Outdated
| # Set working directory | ||
| WORKING_DIR="$(pwd)" | ||
| # Function removed - no longer needed since we use WORKING_DIR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[nitpick] This comment is unclear about which function was removed. Consider being more specific, such as '# get_repo_root function removed - no longer needed since we use WORKING_DIR'.
| #Function removed - no longer needed since we use WORKING_DIR | |
| #get_repo_root function removed - no longer needed since we use WORKING_DIR |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
| # Check git availability | ||
| try { | ||
| $repoRoot=git rev-parse--show-toplevel2>$null | ||
| git rev-parse--show-toplevel2>$null|Out-Null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The git command output is being redirected to Out-Null but the error stream is still redirected to $null. For consistency, consider usinggit rev-parse --show-toplevel >$null 2>&1 or handle both streams uniformly.
| git rev-parse--show-toplevel2>$null|Out-Null | |
| git rev-parse--show-toplevel>$null2>&1 |
Copilot uses AI. Check for mistakes.
| return 1 | ||
| } | ||
| # Resolve repository root. Prefer git information when available, but fall back | ||
| # to searching for repository markers so the workflow still functions in repositories that | ||
| # were initialised with --no-git. | ||
| # Set working directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thefind_repo_root function is now unused but still defined in the file. This dead code should be removed to improve maintainability.
Copilot uses AI. Check for mistakes.
I see a potential flaw here - if we just use the working directory sourced from |
da1z commentedSep 24, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
yes. if vs code opened in wrong working dir i would consider it as user error. and if it is wrong dir opened in vs code then none of / commands would work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md.Create the file orlearn how to get started.
| # Set working directory | ||
| $workingDir=Get-Location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The variable$workingDir should use consistent naming with other scripts. All other files use$WORKING_DIR (uppercase with underscore), but this uses camelCase. Consider changing to$WORKING_DIR = (Get-Location).Path for consistency.
Copilot uses AI. Check for mistakes.
| TEMPLATE="$REPO_ROOT/.specify/templates/spec-template.md" | ||
| TEMPLATE="$WORKING_DIR/.specify/templates/spec-template.md" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
[nitpick] The empty line at line 76 appears to be unnecessary whitespace that doesn't add value to the code structure.
Copilot uses AI. Check for mistakes.
Uh oh!
There was an error while loading.Please reload this page.
opening to start discussion on the approach. let me know what you think