fix(app): scroll active tab into view and validate USERNAME patch paths#1
fix(app): scroll active tab into view and validate USERNAME patch paths#1
Conversation
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
Co-authored-by: Geralt4 <[email protected]>
Geralt4
changed the title
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
# Conflicts: # packages/app/src/pages/session/session-side-panel.tsx
Geralt4
changed the title
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
There was a problem hiding this comment.
Pull request overview
This PR addresses two reported bugs: improving the Session side-panel tab strip auto-scroll so the newly-activated tab is scrolled the minimum distance into view, and adding coverage to ensure apply_patch rejects Windows paths containing a literal USERNAME.
Changes:
- Adjust tab-list scrolling logic to scroll just enough left/right when the active tab is outside the visible area.
- Add a Windows-gated test asserting
apply_patchrejects patch paths containing the literalUSERNAME.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/opencode/test/tool/apply_patch.test.ts | Adds a Windows-only regression test for rejecting USERNAME in patch paths. |
| packages/app/src/pages/session/session-side-panel.tsx | Updates the scroll math to minimally scroll the active tab into view depending on overflow direction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test.if(process.platform === "win32")("rejects Windows path with unexpanded USERNAME on win32", async () => { | ||
| const { ctx } = makeCtx() | ||
| const patch = "*** Begin Patch\n*** Add File: C:\\Users\\USERNAME\\Documents\\file.txt\n+content\n*** End Patch" | ||
| await expect(execute({ patchText: patch }, ctx)).rejects.toThrow( | ||
| 'Path contains "USERNAME" which may indicate an unexpanded', | ||
| ) | ||
| }) |
There was a problem hiding this comment.
bun:test in this repo commonly uses test.skipIf(...) or a runtime if (...) return for conditional execution, but this new test uses test.if(...) (which isn’t used elsewhere in the codebase). If test.if isn’t supported by the Bun version in CI, this will throw at module-eval time and fail the entire test file. Prefer test.skipIf(process.platform !== "win32")(...) (or an early-return inside the test body) to gate this on Windows while keeping the test registered/typed consistently.
| test.if(process.platform === "win32")("rejects Windows path with unexpanded USERNAME on win32", async () => { | |
| const { ctx } = makeCtx() | |
| const patch = "*** Begin Patch\n*** Add File: C:\\Users\\USERNAME\\Documents\\file.txt\n+content\n*** End Patch" | |
| await expect(execute({ patchText: patch }, ctx)).rejects.toThrow( | |
| 'Path contains "USERNAME" which may indicate an unexpanded', | |
| ) | |
| }) | |
| test.skipIf(process.platform !== "win32")( | |
| "rejects Windows path with unexpanded USERNAME on win32", | |
| async () => { | |
| const { ctx } = makeCtx() | |
| const patch = "*** Begin Patch\n*** Add File: C:\\Users\\USERNAME\\Documents\\file.txt\n+content\n*** End Patch" | |
| await expect(execute({ patchText: patch }, ctx)).rejects.toThrow( | |
| 'Path contains "USERNAME" which may indicate an unexpanded', | |
| ) | |
| }, | |
| ) |
Issue for this PR
Fixes anomalyco#11674
Fixes anomalyco#11687
Type of change
What does this PR do?
USERNAMEinapply_patch.How did you verify your code works?
Checklist