Skip to content

Conversation

da1z
Copy link

@da1z da1z commented Sep 23, 2025

opening to start discussion on the approach. let me know what you think

@da1z da1z requested a review from localden as a code owner September 23, 2025 17:43
@Copilot Copilot AI review requested due to automatic review settings September 23, 2025 17:43
Copy link
Contributor

@Copilot Copilot AI left a 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_ROOT variable with WORKING_DIR throughout all scripts
  • Set WORKING_DIR to 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 or learn how to get started.

@da1z
Copy link
Author

da1z commented Sep 23, 2025

fix for #516

@da1z da1z requested a review from Copilot September 23, 2025 18:07
Copy link
Contributor

@Copilot Copilot AI left a 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 or learn how to get started.

@da1z da1z requested a review from Copilot September 23, 2025 18:35
Copy link
Contributor

@Copilot Copilot AI left a 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 or learn how to get started.

}

get_feature_dir() { echo "$1/specs/$2"; }
get_feature_dir() { echo "$WORKING_DIR/specs/$1"; }
Copy link
Preview

Copilot AI Sep 23, 2025

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.

# Set working directory
$script:WorkingDir = (Get-Location).Path

# Function removed - no longer needed since we use WorkingDir
Copy link
Preview

Copilot AI Sep 23, 2025

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'.

Suggested change
# 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.

# Set working directory
WORKING_DIR="$(pwd)"

# Function removed - no longer needed since we use WORKING_DIR
Copy link
Preview

Copilot AI Sep 23, 2025

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'.

Suggested change
# 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.

@da1z da1z requested a review from Copilot September 23, 2025 20:46
Copy link
Contributor

@Copilot Copilot AI left a 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 or learn how to get started.

# Check git availability
try {
$repoRoot = git rev-parse --show-toplevel 2>$null
git rev-parse --show-toplevel 2>$null | Out-Null
Copy link
Preview

Copilot AI Sep 23, 2025

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 using git rev-parse --show-toplevel >$null 2>&1 or handle both streams uniformly.

Suggested change
git rev-parse --show-toplevel 2>$null | Out-Null
git rev-parse --show-toplevel >$null 2>&1

Copilot uses AI. Check for mistakes.

Comment on lines 31 to +34
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
Copy link
Preview

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The find_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.

@localden
Copy link
Collaborator

I see a potential flaw here - if we just use the working directory sourced from pwd, you're relying on the terminal being in the right context. So, if I am in VS Code, and happened to open the wrong folder in the terminal session, this can create the artifacts or look them up in the wrong location?

@da1z
Copy link
Author

da1z commented Sep 24, 2025

I see a potential flaw here - if we just use the working directory sourced from pwd, you're relying on the terminal being in the right context. So, if I am in VS Code, and happened to open the wrong folder in the terminal session, this can create the artifacts or look them up in the wrong location?

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.

@da1z da1z requested a review from Copilot September 26, 2025 12:45
Copy link
Contributor

@Copilot Copilot AI left a 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 or learn how to get started.

Comment on lines +17 to +18
# Set working directory
$workingDir = Get-Location
Copy link
Preview

Copilot AI Sep 26, 2025

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"

Copy link
Preview

Copilot AI Sep 26, 2025

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.

Suggested change

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants