Repository navigation
fix: Cookie Sync path and launch-default issues found in E2E of #235 - #237
Closed
devin-ai-integration[bot] wants to merge 4 commits into
Closed
devin-ai-integration[bot] wants to merge 4 commits into
devin-ai-integration[bot] wants to merge 4 commits into
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
Collaborator
|
Superseded by #239, merged for 2.9.0. The missing caller-relative cookie-path and specific includeSession validation commits were cherry-picked with attribution. Explicit-option precedence and empty-source path/profile redaction had already shipped on main in 2.8.9, so those implementations were retained. Additional regression coverage preserves malformed-path validation across daemon transport. The explicit-path encryption limitation is now documented: use a registered browser source for encrypted Chromium profiles. Closing this conflicting maintenance-branch draft after the replacement landed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Fixes four bugs found during the E2E review of #235. This PR targets
maintenance/2.8.9and is based on reviewed head867468b. Each fix is in its own commit with a regression test.--source-pathresolves against the daemon's directory (src/daemon-client.ts). The session daemon keeps the working directory of whichever CLI spawned it. The relativesource.pathcrossed the socket unchanged, socookies sync --source-path srcprof/Defaultfailed when run from a different folder. Worse, it silently read a differentsrcprof/Defaultthat existed under the daemon's folder. The daemon proxy now resolves a relative path in the caller's process before sending it.includeSessionwith an explicit path reported the wrong error.extractCookieSyncthrew the specific TypeError, butclient.tswraps every extraction error asCookie Sync could not read the selected local browser profile.(reader_unavailable). The check now runs innormalizeCookieSyncOptions, so callers getincludeSession is unavailable for explicit cookie paths.src/launch-config.ts; Greptile P1 on Release BetterWright 2.8.9 #235).resolveLaunchOptionsparsed everyBETTERWRIGHT_*launch variable, even for fields the caller had supplied, sonew BetterWright({ adBlock: false })threw whenBETTERWRIGHT_AD_BLOCK=invalidwas set. The same applied toBETTERWRIGHT_PROFILE,BETTERWRIGHT_HEADLESSandBETTERWRIGHT_DOWNLOAD_POLICY. All of these constructed successfully onmain. Explicitly supplied fields now skip their environment variable. Invalid values still throw when the caller relies on the environment.src/client.ts; Greptile P2 on Release BetterWright 2.8.9 #235). With no matching cookies,syncCookiesreturnedsource: { path: "/home/…/Default" }. Results with matches already return{ path: "selected" }. Empty results now return the same placeholder.Not fixed here, because it needs a maintainer decision: on Linux,
--source-pathcannot read an encrypted (v10) Chromium database. It fails withreader_failed, becausefromPathgets nobrowserId/localStatePath. I reproduced this on867468bwith a synthetic v10-encrypted database.Checklist
bun run release:checkwas not run in full.test:unithas 2 failures on this VM that come from the environment: thechromium-packagepackager tests need Python ≥3.11hashlib.file_digest, and this VM has 3.10. They fail the same way on867468b. lint, typecheck,test:types,check:buildandbuild:harnesspass.types/*.d.ts: no signature change.How it was verified
tests/node/daemon.test.ts(relative path),tests/node/cookie-sync.test.ts(includeSession),tests/node/launch-config.test.ts(env shadowing) andtests/node/client.test.ts(empty-result redaction)./tmp/dirA, thencookies sync --source-path srcprof/Default --domain 127.0.0.1run from the fixture folder:synced: 1, and the local fixture page reports authenticated.includeSession+ path returns the specific error.{"path":"selected"}.Source PR: #235
Link to Devin session: https://app.devin.ai/sessions/93a7cc971a87450b9f748f2b30f84153
Open in Devin Desktop: https://app.devin.ai/desktop/session/93a7cc971a87450b9f748f2b30f84153?variant=devin