test issue#147
Conversation
|
Started — view session to follow along. |
🤖 Augment PR SummarySummary: This PR refactors Changes:
Technical Notes: The new context loader aims to minimize round-trips by running independent git checks in parallel and doing the GitHub API call only after org/repo are known. 🤖 Was this summary useful? React with 👍 or 👎 |
| // getParentRepoByOrgRepo fetches the parent repository full name via the GitHub API. | ||
| func getParentRepoByOrgRepo(wd, org, repo string) (string, error) { | ||
| stdout, stderr, err := new(exec.PipedExec). | ||
| Command("gh", "api", "repos/"+org+slash+repo, "--jq", ".parent.full_name"). |
There was a problem hiding this comment.
gitcmds/gitcmds.go:485: Using --jq ".parent.full_name" can return the literal string null for non-fork repos, which would make len(parentRepo) > 0 checks mis-detect a fork and potentially break single-remote workflows.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| oldSize = newFileSize | ||
| case `??`: | ||
| newFileSize, err2 = getFileSize(wd, name) | ||
| case `AD`: |
There was a problem hiding this comment.
gitcmds/status.go:168: Treating status AD as "never existed" and continue may hide a real staged add (index) vs deleted working-tree state, causing changed-file accounting to be silently wrong for that edge case.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| b.Helper() | ||
| out, err := exec.Command("git", "rev-parse", "--show-toplevel").Output() | ||
| require.NoError(b, err) | ||
| return string(out[:len(out)-1]) // trim trailing newline |
There was a problem hiding this comment.
internal/commands/dev_test.go:27: string(out[:len(out)-1]) assumes non-empty output and \n line endings; on Windows (\r\n) or unexpected output this can leave a trailing \r or panic, which makes the benchmark brittle.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| oldSize = newFileSize | ||
| case `??`: | ||
| newFileSize, err2 = getFileSize(wd, name) | ||
| case `AD`: |
There was a problem hiding this comment.
AD means the blob is still staged even though the working-tree file was deleted, so continue hides content that would still be included by git commit. This makes qs status omit the staged file from the positive-delta summary instead of reporting the indexed addition.
Severity: low
🤖 Was this useful? React with 👍 or 👎
test issue