Repository navigation
fix(core): normalize session directory lookup separators - #53924
poijygfdyy wants to merge 20 commits into
Conversation
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
github-actions
Bot
added
the
needs:compliance
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 72 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
I checked this against a live serve from the base and the PR: with the PR, GET /api/session?directory=<dir>/ (and <dir>//) now finds sessions stored as <dir>, so #53735 is fixed. Type check and session-store.test.ts pass, and the new test fails on the base as it should.
One regression: session creation doesn't normalize the directory, so POST /api/session with location.directory: "<dir>/" stores <dir>/. Before this PR, that session could be listed with ?directory=<dir>/. With the PR, the lookup value is trimmed but the saved value isn't, so the session can't be listed under any form of its path (checked live). Trimming only one side of the comparison causes this. Please normalize in one place that covers both saving and lookup (see the inline comment), and extend the test with a session created with a trailing separator.
| const conditions: SQL[] = [] | ||
| if ("directory" in input) conditions.push(eq(SessionTable.directory, input.directory)) | ||
| if ("directory" in input) { | ||
| const trimmed = input.directory.replace(/[\\/]+$/, "") |
There was a problem hiding this comment.
This trims only the lookup value. Session creation saves location.directory as given (canonical leaves it unchanged on POSIX, and path.normalize keeps a trailing separator on Windows). So a session created with directory: "/x/" is saved as /x/, and with this change no query finds it anymore. I checked this live: before the change, ?directory=/x/ returned it.
Normalizing in directoryColumn.toDriver (packages/core/src/database/path.ts) would cover both saving and lookup, because drizzle's eq() passes its value through the column's toDriver. It would also sit next to the existing Windows separator handling. Normalizing where the location is canonicalized would work too.
Smaller point: [\\/]+$ also strips trailing backslashes on Linux and macOS, where \ is a legal file-name character, so it may be worth limiting that to Windows.
poijygfdyy
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. |
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 72-hour window. Feel free to open a new pull request that follows our guidelines. |
github-actions
Bot
removed
the
needs:compliance
1 participant
Issue for this PR
Closes #
Type of change
What does this PR do?
Please provide a description of the issue, the changes you made to fix it, and why they work. It is expected that you understand why your changes work and if you do not understand why at least say as much so a maintainer knows how much to value the PR.
If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!
How did you verify your code works?
Screenshots / recordings
If this is a UI change, please include a screenshot or recording.
Checklist
If you do not follow this template your PR will be automatically rejected.