Repository navigation
Conversation
Every push packs the source profile's Local Storage leveldb into the envelope, and the watcher also pushes on Local Storage writes. A Linux sink (or any sink with skip_chrome_sqlite) never applies it. On one Mac the directory was 64 MB, so each push was ~90 MB (150 GB uploaded in 16.5 h), and slow uplinks overran the watcher's 60 s push budget. --skip-local-storage (or AGENTCOOKIE_SKIP_LOCAL_STORAGE=1 for a LaunchAgent) skips the pack and stops watching the directory. Default behavior, source.yaml keys and the wire format are unchanged. Refs mvanhorn#149
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d269a9b51d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| t.Fatal(err) | ||
| } | ||
|
|
||
| if _, err := fx.push(); err != nil { |
There was a problem hiding this comment.
When the suite is run with the newly documented AGENTCOOKIE_SKIP_LOCAL_STORAGE=1 already exported, this first “default” push also skips packing because skipLocalStorage() reads the inherited process environment, so the assertion that the tarball is present fails before t.Setenv changes anything. Clear the variable at the start of the test (then set it to 1 for the second push) so users or CI exercising this environment-based feature can still run the suite.
Useful? React with 👍 / 👎.
…ndexedDB Addresses review on mvanhorn#150: the default-push case clears an inherited AGENTCOOKIE_SKIP_LOCAL_STORAGE, the test compares whole Cookies slices instead of hosts, and the FAQ notes IndexedDB stays opt-in.
|
Thanks for the reviews. dba41c2 addresses all three: the default-push case now clears an inherited |
|
Thanks @Dannyxlm, this is a great find and a clean fix. Your two commits are cherry-picked onto #153 with your authorship kept, and you are credited as co-author. That PR also adds |
…secrets (#153) * feat(source): add --skip-local-storage to stop shipping Local Storage Every push packs the source profile's Local Storage leveldb into the envelope, and the watcher also pushes on Local Storage writes. A Linux sink (or any sink with skip_chrome_sqlite) never applies it. On one Mac the directory was 64 MB, so each push was ~90 MB (150 GB uploaded in 16.5 h), and slow uplinks overran the watcher's 60 s push budget. --skip-local-storage (or AGENTCOOKIE_SKIP_LOCAL_STORAGE=1 for a LaunchAgent) skips the pack and stops watching the directory. Default behavior, source.yaml keys and the wire format are unchanged. Refs #149 * test(source): compare full cookie sets, isolate env; clarify FAQ on IndexedDB Addresses review on #150: the default-push case clears an inherited AGENTCOOKIE_SKIP_LOCAL_STORAGE, the test compares whole Cookies slices instead of hosts, and the FAQ notes IndexedDB stays opt-in. * feat(source): add --skip-secrets for a cookie-only source A normal source always loaded the secrets bus and shipped it to every push sink and, through GET /pull, to every paired teammate. Owners who keep tokens in a password manager had no way to say "never ship secrets from this source" short of running a CDP source. agentcookie source --skip-secrets (or AGENTCOOKIE_SKIP_SECRETS) skips loadSecretsPayload on every push and, in --watch, starts neither the secrets-bus watcher nor the discovery watcher, so no manifest or carried file is read. The env var fails closed: any value other than empty or 0 turns it on, and an unusual value warns once. Source state records whether the last push skipped secrets and how many CLIs it shipped, and doctor reports it as "Secrets push". Docs cover the cookie-only recipe and note that the switch does not remove secrets already delivered. Requested by @jnyross in #151. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN * refactor(source): tidy the --skip-secrets switch and doctor wording Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN * fix(source): keep the secrets summary on dry runs; move skip switches A --dry-run cycle sends nothing, but it overwrote the recorded secrets summary, so doctor could say "shipping N cli(s)" for secrets that never left the machine. Dry runs now leave it alone. The skip switch helpers move to source_skip.go to keep source.go from growing. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN * fix(source): keep the secrets summary when a cycle fails before publishing Addresses review feedback on #153. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN * fix(source): carry the saved secrets summary into a new source run Addresses review feedback on #153. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LgVWtkXgDGWMh6L95d8piN --------- Co-authored-by: Dannyxlm <[email protected]> Co-authored-by: Matt Van Horn <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
Summary
Adds
agentcookie source --skip-local-storage, orAGENTCOOKIE_SKIP_LOCAL_STORAGE=1for a LaunchAgent. With either set, pushes stop packing the browser profile's Local Storage leveldb, and the watcher stops watching that directory. A Linux sink (or any sink withskip_chrome_sqlite) never applies Local Storage. On our Mac it was 64 MB, so each push was ~90 MB: 150 GB uploaded in 16.5 h, pluscontext deadline exceededon slower uplinks. With the flag, each push is ~1.8 MB and the sidecar receives the same cookies.This is part 1 of #149.
Verified
go build ./...go vet ./...go test ./...(all 26 packages), plusgo test -race ./internal/cli/ ./internal/config/golangci-lint run ./internal/cli/...(v2.11.4): 0 issuesTestSourcePushLocalStorageOptOut:LocalStorageTarballis empty and the cookie set is identical.browser.profile), on a v1.3.0 Mac source → v1.3.0 Linux sink.nettopshowed ~90 MB → ~1.8 MB per push, with 0 failures after the switch.Notes
source.yamlkey. I first triedlocal_storage: false, butTestSourceConfigYAMLKeysUnchangedForOlderBinaries(and strict decoding in older binaries) argue against new keys. So this follows the--skip-dbsc-suspect/AGENTCOOKIE_SKIP_DBSC_SUSPECTpattern instead.LocalStorageTarballis simply empty.(Prepared with my AI agent, Ava, while debugging our setup.)