Improve Where Was I worktree context and Git history - #2342
Improve Where Was I worktree context and Git history#2342jamesmontemagno wants to merge 14 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d224e01-e53f-46f8-beb2-0b8b64c5ee89
|
🔴 Contributor Reputation Check: HIGH risk
Maintainers: please review this contributor before merging. |
There was a problem hiding this comment.
Pull request overview
Enhances the Where Was I canvas with richer Git worktree context and diff previews.
Changes:
- Adds Git history, divergence, and worktree-change collection.
- Adds interactive diff previews and expanded resume context.
- Adds integration tests and releases version 1.1.0.
Show a summary per file
| File | Description |
|---|---|
extensions/where-was-i/package.json |
Updates package version. |
extensions/where-was-i/git-context.test.mjs |
Tests Git context and diff collection. |
extensions/where-was-i/git-context.mjs |
Implements Git context and file diffs. |
extensions/where-was-i/extension.mjs |
Adds graph, diff drawer, and resume behavior. |
extensions/where-was-i/.github/plugin/plugin.json |
Updates extension metadata version. |
.github/plugin/marketplace.json |
Publishes the updated marketplace version. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 5
- Review effort level: Medium
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c161d734-edb5-4707-a48c-966d4e83a2cf
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c161d734-edb5-4707-a48c-966d4e83a2cf
🔒 PR Risk Scan ResultsScanned 6 changed file(s).
|
There was a problem hiding this comment.
Review details
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
extensions/where-was-i/extension.mjs:697
- Splitting at the first hash outside
branchHashescan hide branch work. When no base is resolved (for example, a local-onlymainrepository),branchHashesis empty and the entire graph is mislabeled and collapsed as base history; merge graphs can also interleave a base commit before later branch commits. Only split when a base exists, and split after the last branch-specific commit so branch work remains visible.
const firstBaseCommit = graph.findIndex(row => row.hash && !branchHashes.has(row.hash));
const focusedGraph = firstBaseCommit >= 0 ? graph.slice(0, firstBaseCommit) : graph;
const baseGraph = firstBaseCommit >= 0 ? graph.slice(firstBaseCommit) : [];
extensions/where-was-i/git-context.mjs:126
- Every regular untracked file is emitted as mode
100644, so an executable script is presented with an inaccurate patch. Derive the new-file mode from the executable bits infileStat.mode.
This issue also appears on line 135 of the same file.
return renderNewFilePatch(relativePath, "100644", addedLines);
extensions/where-was-i/extension.mjs:844
- Concurrent diff requests can complete out of order. If a user opens file A and then file B before A finishes, A's later response overwrites B's badge and content while the title still says B. Abort the previous request or assign each request a sequence ID and ignore stale responses before updating the dialog.
try {
const res = await fetch("/file-diff?path=" + encodeURIComponent(path));
const data = await res.json();
if (!res.ok) throw new Error(data.error || "Unable to load this diff.");
const status = describeStatus(data.code);
extensions/where-was-i/git-context.mjs:135
- A porcelain status can contain two records for the same path—for example,
git rm --cached fileleaves both a staged deletion and an untrackedfile. This path-onlyfindalways returns the first record, so the untracked row's diff button shows the staged deletion and the new file contents cannot be previewed. Preserve the selected record's status/identity through the UI and endpoint, or combine same-path records before lookup.
const entry = parseStatusOutput(status).find((item) => item.path === relativePath);
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c161d734-edb5-4707-a48c-966d4e83a2cf
There was a problem hiding this comment.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
extensions/where-was-i/extension.mjs:713
--topo-orderdoes not guarantee that every HEAD-only commit appears before commits reachable only frombaseRef; a newer base commit can occur first. Splitting at the first non-branch hash can therefore put later worktree commits inside the collapsed “base” section. Collapse only a suffix that starts after the final branch hash (and add a behind/diverged graph case to the test).
const firstBaseCommit = canSplitGraph
? graph.findIndex(row => row.hash && !branchHashes.has(row.hash))
: -1;
const focusedGraph = firstBaseCommit >= 0 ? graph.slice(0, firstBaseCommit) : graph;
const baseGraph = firstBaseCommit >= 0 ? graph.slice(firstBaseCommit) : [];
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
…edge cases - Remove inline onclick handlers; PR/issue cards are buttons wired with addEventListener from data, the refresh/resume buttons use ids, and the page is served with a nonce-based CSP (script-src 'nonce-...', default-src 'none'). escapeHtml now also escapes apostrophes. - Collapse only the graph suffix after the final branch commit (splitCommitGraph, computed server-side as baseGraphStart) so --topo-order interleaving never hides worktree commits. - Preserve the selected status record through the UI, /file-diff, and get_file_diff so a path that appears twice (e.g. staged deletion + untracked re-creation) previews the right record. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 18350039-5e2b-40f0-b537-bc22cabcbb1b
There was a problem hiding this comment.
Review details
Suppressed comments (5)
Previously missed (2) — in code that hasn't changed since the last review.
extensions/where-was-i/extension.mjs:283
--azurehas only about 2.8:1 contrast against the white/azure-tinted row background. These graph characters communicate topology and need at least 3:1 even when treated as non-text graphics (and 4.5:1 if treated as text); use a darker blue.
This issue also appears in the following locations of the same file:
- line 312
- line 380
color: var(--azure);
extensions/where-was-i/extension.mjs:290
--metais approximately 2.6:1 against the card background, so these small commit hashes do not meet the 4.5:1 text contrast requirement. Use a darker metadata color here.
This issue also appears on line 665 of the same file.
color: var(--meta);
extensions/where-was-i/extension.mjs:312
- The azure text on
--azure-tintis only about 2.5:1 contrast, below the 4.5:1 requirement for this 0.65rem ref label. Please use a darker foreground.
color: var(--azure);
extensions/where-was-i/extension.mjs:380
- This newly added rename badge repeats the same low-contrast azure-on-tint combination (about 2.5:1), making the small status text difficult to read. Use a darker blue foreground.
.status-badge.renamed { background: var(--azure-tint); color: var(--azure); }
extensions/where-was-i/extension.mjs:667
- Checking
RandAbefore a worktree deletion mislabels validRDandADstates as “Renamed + edited” or “Added + edited.” Handle aDworktree column first so the canvas does not hide that the file was deleted after staging.
if (index === "R") return { label: worktree === " " ? "Renamed" : "Renamed + edited", kind: "renamed" };
if (index === "A") return { label: worktree === " " ? "Staged add" : "Added + edited", kind: "added" };
if (index === "D" || worktree === "D") return { label: index !== " " && worktree !== " " ? "Staged + deleted" : "Deleted", kind: "deleted" };
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Addressed the two additional findings listed only in the reviewer’s suppressed-comments summary (so they did not have replyable inline threads) in
The title-injection/CSP finding was also fixed in |
Summary
Validation
node --test extensions/where-was-i/git-context.test.mjsnpm run buildnpm run plugin:validate -- --plugin where-was-i