Skip to content

fix(manual): say that Chrome capture starts with a fresh profile - #157

Closed
namanarora00 wants to merge 1 commit into
kalil0321:mainfrom
namanarora00:fix/manual-chrome-profile-message
Closed

namanarora00 wants to merge 1 commit into
kalil0321:mainfrom
namanarora00:fix/manual-chrome-profile-message

Conversation

@namanarora00

@namanarora00 namanarora00 commented Nov 2, 2026 •

Copy link
Copy Markdown

Why

Manual capture in Chrome prints using real chrome (profile copy), and ManualBrowser's docstring says it uses "your actual Chrome browser with existing profile". It doesn't. Since _start_with_real_chrome was added (ae0206f), it has launched Chrome with launch_persistent_context on an empty tempfile.mkdtemp() directory. The user's profile directory is only checked to see that Chrome is installed, and nothing is copied from it. Users who expect their logins and cookies find themselves signed out, with a message that suggests the opposite.

What

  • The console message now says the capture uses Chrome with a fresh profile, and to sign in during the capture if the site needs it.
  • The class and method docstrings describe what the code does.
  • The manual-mode docs page says the browser starts without your existing logins.
  • New tests/test_manual_browser_profile.py pins the behavior: the browser runs on the Chrome channel, user_data_dir is not the user's profile, nothing is written into the user's profile, and the message says "fresh profile" rather than "profile copy".
  • CHANGELOG entry under Unreleased → Fixed.

I left out actually copying the profile on purpose. A Chrome profile is often gigabytes, it is locked while Chrome runs, and copying it would duplicate the user's cookies and saved credentials into a temp directory. Cookie decryption on the copy also depends on the OS. If you want that behavior, I'd rather discuss it separately.

Verification

  • The new test fails on main on the profile copy message and passes with the fix.
  • pytest: 1232 passed, 10 skipped (1231 on main, plus 1 new). ruff check src tests and mypy src are clean.

Each of the three PRs I'm opening (this one, the list/show cost fix and the OpenCode cost fix) adds lines in the same CHANGELOG section. Whichever merges second will need a small conflict resolution there.


Summary by cubic

Fixes the misleading message and docs for manual Chrome capture, which said it used a "profile copy" of your Chrome but has always launched Chrome with a fresh, empty temporary profile. The message, docstrings, and manual-mode docs now say the capture starts signed out and to sign in during the capture.

  • A new test pins the fresh-profile behavior and the wording.
  • Copying the real profile is intentionally left out: it is large, locked while Chrome runs, and duplicating cookies and saved credentials into a temp directory is undesirable.

Written for commit 5de50ab. Summary will update on new commits.

View guided diff

RetriggerView in GreptileConfidence Score: 4/5

No blocking issue was established; the remaining regression-test coverage gap does not prevent merging.

Findings

  1. P2 Profile copying goes undetected ▶
Fix with agent prompt
### Issue 1
tests/test_manual_browser_profile.py:17-18
The new test cannot detect copying the user's profile because `user_profile` is empty. Copying its contents into the separate launch directory still passes every assertion; an in-memory mutation confirmed this. This is a non-blocking coverage gap: a future change could copy existing logins while the test passes and the message still promises a fresh profile.

Add a synthetic cookie or settings file, assert that the source remains unchanged, and inspect `user_data_dir` inside the mocked launch call to check that the file was not copied. Check there because the temporary directory is deleted before the method returns.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR corrects manual capture instructions without changing how Chrome launches. The console message and documentation now explain that capture starts without existing logins, and the new test checks the Chrome channel, separate profile directory, and corrected message.

One non-blocking test gap remains in tests/test_manual_browser_profile.py: the empty source profile cannot establish that existing profile files are never copied. The test still passed when profile copying was introduced in memory. This concern does not prevent merging.

Intentional scope: namanarora00 explicitly deferred actually copying the user's Chrome profile because profiles can be large and locked, copying would duplicate cookies and saved credentials into temporary storage, and cookie decryption depends on the operating system.

  • Manual Chrome capture now says it starts with a fresh profile.

Reviews (1) · Last reviewed commit: "fix(manual): say that Chrome capture sta..." · Reviewed by Greptile

Manual capture printed "using real chrome (profile copy)" and the class
docstring promised the user's existing profile, but _start_with_real_chrome
has always launched Chrome on an empty temporary profile: the user's profile
directory is only checked to see that Chrome is installed. Users expecting
their logins found themselves signed out with no explanation.

Make the message, docstrings and manual-mode docs say the browser starts
with a fresh profile and that sign-in happens during the capture. Copying the
real profile is left out on purpose: it is large, locked while Chrome runs,
and would duplicate the user's cookies and saved credentials into a temp dir.
@namanarora00
namanarora00 force-pushed the fix/manual-chrome-profile-message branch from 9f0eead to 5de50ab Compare November 2, 2026 14:33

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

View guided diff | Re-trigger cubic

Comment on lines +17 to +18
user_profile = tmp_path / "Chrome"
user_profile.mkdir()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Profile copying goes undetected

The new test cannot detect copying the user's profile because user_profile is empty. Copying its contents into the separate launch directory still passes every assertion; an in-memory mutation confirmed this. This is a non-blocking coverage gap: a future change could copy existing logins while the test passes and the message still promises a fresh profile.

Add a synthetic cookie or settings file, assert that the source remains unchanged, and inspect user_data_dir inside the mocked launch call to check that the file was not copied. Check there because the temporary directory is deleted before the method returns.

Artifacts

Evidence from the check

  • The executed shell runner records the original pytest result and both runtime comparisons, making the validation reproducible.

Evidence from the check

  • The executed Python source injects copying only in memory and observes launch-directory contents, demonstrating the coverage gap without editing tracked files.

Command output from the check

  • The original targeted pytest test was executed without mutation and passed, establishing the baseline.

Command output from the check

  • The unchanged method was executed with empty and populated source profiles, showing an empty launch destination in both cases.

Command output from the check

  • The mutated method passed the original test and copied the populated fixture into the observed launch destination, confirming the missing destination-content check.

View artifacts

TREX Ran code and verified through TREX

Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_manual_browser_profile.py
Line: 17-18

Comment:
**Profile copying goes undetected**

The new test cannot detect copying the user's profile because `user_profile` is empty. Copying its contents into the separate launch directory still passes every assertion; an in-memory mutation confirmed this. This is a non-blocking coverage gap: a future change could copy existing logins while the test passes and the message still promises a fresh profile.

Add a synthetic cookie or settings file, assert that the source remains unchanged, and inspect `user_data_dir` inside the mocked launch call to check that the file was not copied. Check there because the temporary directory is deleted before the method returns.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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