Resolve update-command-spec-version repo paths dynamically#290
Conversation
The update-command-spec-version skill scripts hardcoded ~/warp and ~/command-signatures, so they only worked when both repos happened to be checked out at those exact paths. Resolve both dynamically instead: - command-signatures is inferred from the script location via git rev-parse --show-toplevel (the scripts live inside the repo). - warp is taken from $WARP_DIR when set, else assumed to be a sibling of command-signatures. Shared resolution lives in scripts/common.sh (sourced by the bash scripts) and is mirrored in list_merged_prs.py. ensure_repos.sh now only fetches the command-signatures checkout it runs from rather than switching branches / pulling into it. Co-Authored-By: Oz <oz-agent@warp.dev>
|
@warp-dev-github-integration[bot] I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR removes hardcoded ~/warp and ~/command-signatures assumptions by adding shared repo-path resolution and wiring the update-command-spec-version scripts to use it. The path resolution approach is reasonable, and I did not find security issues or spec-drift concerns because no approved/repository spec context was provided.
Concerns
ensure_repos.shnow avoids mutating the command-signatures working tree, but its genericgit fetch origincalls do not guarantee the specific remote refs later consumed byget_latest_hash.shandget_current_hash.share present or fresh in branch-limited/shallow checkouts.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| echo "Fetching command-signatures (${CMD_SIGS_DIR})..." >&2 | ||
| git -C "${CMD_SIGS_DIR}" fetch origin | ||
|
|
||
| if [ -d "${WARP_DIR}" ]; then | ||
| echo "Fetching warp..." >&2 | ||
| echo "Fetching warp (${WARP_DIR})..." >&2 | ||
| git -C "${WARP_DIR}" fetch origin |
There was a problem hiding this comment.
git fetch origin follows the checkout's configured refspec, so branch-limited or shallow checkouts can leave origin/main or origin/master missing/stale.
| echo "Fetching command-signatures (${CMD_SIGS_DIR})..." >&2 | |
| git -C "${CMD_SIGS_DIR}" fetch origin | |
| if [ -d "${WARP_DIR}" ]; then | |
| echo "Fetching warp..." >&2 | |
| echo "Fetching warp (${WARP_DIR})..." >&2 | |
| git -C "${WARP_DIR}" fetch origin | |
| echo "Fetching command-signatures (${CMD_SIGS_DIR})..." >&2 | |
| git -C "${CMD_SIGS_DIR}" fetch origin main:refs/remotes/origin/main | |
| if [ -d "${WARP_DIR}" ]; then | |
| echo "Fetching warp (${WARP_DIR})..." >&2 | |
| git -C "${WARP_DIR}" fetch origin master:refs/remotes/origin/master |
There was a problem hiding this comment.
Done in e9cdee2 — ensure_repos.sh now fetches the exact refs the later scripts read: git -C "${CMD_SIGS_DIR}" fetch origin main:refs/remotes/origin/main and git -C "${WARP_DIR}" fetch origin master:refs/remotes/origin/master, so origin/main / origin/master stay fresh even in branch-limited or shallow checkouts.
Address review feedback: a bare 'git fetch origin' follows the checkout's configured refspec, so branch-limited or shallow clones could leave origin/main / origin/master (read later by get_latest_hash.sh and get_current_hash.sh) missing or stale. Fetch each exact ref explicitly. Co-Authored-By: Oz <oz-agent@warp.dev>
Drop the references to the previous hardcoded ~/warp / ~/command-signatures paths from the code comments; comments now describe only current behavior. Co-Authored-By: Oz <oz-agent@warp.dev>
## Description Updates `warp-command-signatures` to ec1ae8e8. ### Merged PRs - Fix git status parser to handle paths with spaces and renames (warpdotdev/command-signatures#272) - fix(git): complete nested untracked files for git add (warpdotdev/command-signatures#279) - Resolve update-command-spec-version repo paths dynamically (warpdotdev/command-signatures#290) ## Changelog Entries for Stable CHANGELOG-BUG-FIX: Fixed `git status` completion parsing for paths with spaces and renames, and completion of nested untracked files for `git add`. <!-- factory-client: {"source":"factory-client","slack_channel":"C0BCE7AELJ2","slack_thread_ts":"1783976762.109799","slack_permalink":"https://warpdev.slack.com/archives/C0BCE7AELJ2/p1783976762109799?thread_ts=1783976762.109799&cid=C0BCE7AELJ2","oz_run_id":"019f5d4d-27c2-750d-823f-90a8b0a92b9c","repo":"warpdotdev/warp"} --> _Conversation: https://staging.warp.dev/conversation/b34001b4-4abb-4cb1-be9d-98f75676605e_ _Run: https://oz.staging.warp.dev/runs/019f5d4d-27c2-750d-823f-90a8b0a92b9c_ _This PR was generated with [Oz](https://warp.dev/oz)._ Co-authored-by: Oz <oz-agent@warp.dev>
Description
The
update-command-spec-versionskill's scripts hardcoded fixed repolocations (
~/warpand~/command-signatures), so they only worked when bothrepos happened to be checked out at those exact paths and failed everywhere
else. This resolves both repos dynamically instead.
Changes
scripts/common.sh, sourced by every bash script, which resolves:git rev-parse --show-toplevel(the scripts live inside the repo).$WARP_DIRwhen set, otherwise a sibling of thecommand-signatures repo (
<parent>/warp).list_merged_prs.py.ensure_repos.shnow only fetches the command-signatures checkout it runsfrom (it no longer
checkout main+pulls into the user's working tree,which is unsafe now that it operates on the real checkout rather than a
throwaway clone); warp is still cloned if missing, else fetched.
SKILL.md.Validation
bash -non all shell scripts +py_compileonlist_merged_prs.py: pass.common.shresolves correctly both with$WARP_DIRset and unset (fallingback to the sibling path).
get_latest_hash.shreturns the command-signaturesorigin/mainhash.get_current_hash.shreads the warporigin/master:Cargo.tomlrev via thesibling-resolved
WARP_DIR(previously would have failed on~/warp).list_merged_prs.pyresolves its repo dir and runs.Conversation: https://staging.warp.dev/conversation/b34001b4-4abb-4cb1-be9d-98f75676605e
Run: https://oz.staging.warp.dev/runs/019f5d4d-27c2-750d-823f-90a8b0a92b9c
This PR was generated with Oz.