Skip to content

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
maintenance/2.8.9from
devin/e2e-fix/235-867468b
Closed

devin-ai-integration[bot] wants to merge 4 commits into
maintenance/2.8.9from
devin/e2e-fix/235-867468b

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

What and why

Fixes four bugs found during the E2E review of #235. This PR targets maintenance/2.8.9 and is based on reviewed head 867468b. Each fix is in its own commit with a regression test.

  1. A relative --source-path resolves against the daemon's directory (src/daemon-client.ts). The session daemon keeps the working directory of whichever CLI spawned it. The relative source.path crossed the socket unchanged, so cookies sync --source-path srcprof/Default failed when run from a different folder. Worse, it silently read a different srcprof/Default that existed under the daemon's folder. The daemon proxy now resolves a relative path in the caller's process before sending it.
  2. includeSession with an explicit path reported the wrong error. extractCookieSync threw the specific TypeError, but client.ts wraps every extraction error as Cookie Sync could not read the selected local browser profile. (reader_unavailable). The check now runs in normalizeCookieSyncOptions, so callers get includeSession is unavailable for explicit cookie paths.
  3. Invalid environment values block explicit constructor options (src/launch-config.ts; Greptile P1 on Release BetterWright 2.8.9 #235). resolveLaunchOptions parsed every BETTERWRIGHT_* launch variable, even for fields the caller had supplied, so new BetterWright({ adBlock: false }) threw when BETTERWRIGHT_AD_BLOCK=invalid was set. The same applied to BETTERWRIGHT_PROFILE, BETTERWRIGHT_HEADLESS and BETTERWRIGHT_DOWNLOAD_POLICY. All of these constructed successfully on main. Explicitly supplied fields now skip their environment variable. Invalid values still throw when the caller relies on the environment.
  4. An empty Cookie Sync result leaked the absolute source filename (src/client.ts; Greptile P2 on Release BetterWright 2.8.9 #235). With no matching cookies, syncCookies returned source: { 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-path cannot read an encrypted (v10) Chromium database. It fails with reader_failed, because fromPath gets no browserId/localStatePath. I reproduced this on 867468b with a synthetic v10-encrypted database.

Checklist

  • bun run release:check was not run in full. test:unit has 2 failures on this VM that come from the environment: the chromium-package packager tests need Python ≥3.11 hashlib.file_digest, and this VM has 3.10. They fail the same way on 867468b. lint, typecheck, test:types, check:build and build:harness pass.
  • Guard proxy: unchanged.
  • No secret output: fix 4 removes a local path from a result envelope.
  • Dependency pins: unchanged.
  • Public API / types/*.d.ts: no signature change.
  • CI job names / actions: unchanged.
  • No unit test imports the worker.
  • CHANGELOG: not updated. These fix behavior that 2.8.9 introduces and has not shipped yet.
  • Nothing private committed. All test cookies are synthetic.

How it was verified

  • Each new or updated test fails without its source change and passes with it: tests/node/daemon.test.ts (relative path), tests/node/cookie-sync.test.ts (includeSession), tests/node/launch-config.test.ts (env shadowing) and tests/node/client.test.ts (empty-result redaction).
  • Real flows with the built CLI/SDK and BetterChromium 153.0.8010.36:
    • Daemon started in /tmp/dirA, then cookies sync --source-path srcprof/Default --domain 127.0.0.1 run from the fixture folder: synced: 1, and the local fixture page reports authenticated.
    • SDK includeSession + path returns the specific error.
    • SDK with invalid env plus explicit options constructs.
    • Empty result source is {"path":"selected"}.
  • Environment: Ubuntu 22.04 x64, Bun 1.4.0.

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

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>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@SSHdotCodes

Copy link
Copy Markdown
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant