Skip to content

Fix Google self-heal: drop impossible messages.google.com:OSID requirement, persist rotated cookies - #148

Merged
MaxGhenis merged 2 commits into
mainfrom
claude/beautiful-hertz-fb2a1c
Jul 22, 2026
Merged

Fix Google self-heal: drop impossible messages.google.com:OSID requirement, persist rotated cookies#148
MaxGhenis merged 2 commits into
mainfrom
claude/beautiful-hertz-fb2a1c

Conversation

@MaxGhenis

Copy link
Copy Markdown
Owner

The bug (live install, reproduced 4×)

A fresh Google re-pair connects, holds ~3-4 minutes, then flips to auth_expired / needs_repair with "Google Messages session cookie expired; refreshing and reconnecting…" and loops there forever. Re-pairing buys minutes, never a fix.

Root cause (code-verified, then proven live)

  1. Impossible repair requirement. The watchdog's credential repair (internal/googlecookies.LoadChromeCookies) hard-required a messages.google.com:OSID Chrome cookie. OSID is a per-subdomain Google service cookie — it exists only if the user has opened Messages-for-web in that Chrome profile. On profiles that never did (zero messages.google.com rows), every repair failed with missing required cookies: messages.google.com:OSIDFlagGoogleNeedsRepair → backoff → retry → forever. Same impossible requirement in both refresh scripts.
  2. Stale cookie snapshots. libgm rotates cookies on every response (UpdateCookiesFromResponse) but the app persisted them only on AuthTokenRefreshed (~daily), so each generation/restart resumed from pair-time cookies.

Live evidence (no re-pair, probe against a copy of the real session)

  • With only fresh .google.com account cookies (SID/HSID/SSID/APISID/SAPISID + SAPISIDHASH), /web/config and the RegisterRefresh RPC return 200 and mint fresh 24h tachyon tokens — the "dead" session revived instantly.
  • Durability run (60s cycles): 14 clean minutes, then HTTP 401 SESSION_COOKIE_INVALID while the config GET still returned 200 — Google invalidates replayed browser-cookie snapshots once Chrome advances the account session. Fresh-pair sessions died in ~3-4 min.
  • Conclusion: durable health = the watchdog heal cycle (die → refresh from Chrome → reconnect, ~every 15 min). This PR unblocks exactly that loop; the supervisor already resets its same-fingerprint streak on liveness, so heal cycles never trip the circuit breaker.

Changes

  • internal/googlecookies: require only the five .google.com account cookies; carry messages.google.com OSID when present (preferred, never required); deterministic host tie-break (ORDER BY host_key, name); truthful docs.
  • scripts/refresh-google-session-cookies-{macos,linux}.py: mirrored.
  • internal/client/events.go: throttled persistence of rotated cookies (sha256 change detection, 5-min window, injectable clock for tests); CookiesLock held around session marshals (pre-existing race).
  • docs/agent-runbook.md: self-healing section rewritten to the proven mechanics (revive, don't re-pair).

Verification

  • go test -race ./internal/googlecookies/ ./internal/client/ ./internal/bridgeadapters/google/ ./internal/app/ ✅; go test ./cmd/ ✅; go vet ./... ✅; go build ./... ✅; both scripts py_compile
  • Counterfactual: reverting the requiredCookies change makes TestLoadChromeCookiesDoesNotRequireMessagesOSID fail; restoring passes.
  • Live-install deploy + ≥15 min /api/status watch: in progress, will report on this PR.

Coordination note: deliberately scoped away from internal/bridge/ and the supervisor (in-flight refactor); PR #145 untouched.

🤖 Generated with Claude Code

MaxGhenis added a commit that referenced this pull request Jul 22, 2026
Adversarial review of the OSID self-heal fix flagged the one real
operational risk: googleCredentialRepairer.RepairCredentials has no rate
limit, and the supervisor's 5-same-fingerprint breaker resets on any
successful activity (resetFailureHistory on reaching Online). So if
Google ever revokes the exported cookie session on a short (~minutes)
cadence, connect -> brief activity -> 401 -> repair -> reconnect could
churn indefinitely and risk account-level over-connect throttling.

Add a cooldown that paces automatic repairs to >= a minimum interval
(default 90s, override via OPENMESSAGE_REPAIR_MIN_INTERVAL). The wait
runs on the parent context BEFORE the 20s refresh-timeout context, so a
too-soon repair is delayed rather than hard-parked (parking would strand
the session: Blocked is terminal with no auto-retry). Transparent to
legitimate expiry, which is measured at >= ~14 min apart. A pacedCount
(exposed via PacedRepairCount) increments on each delayed repair and
doubles as the "fast-revocation is actually happening" signal for the
durability watch. ctx cancellation during the wait returns without
flagging repair or refreshing (it's a shutdown, not a failure).

Scope: cmd/google_supervisor.go only; internal/bridge untouched. Tests
cover first-repair-immediate, pace-too-soon (>= interval + count),
after-interval-immediate, and cancel-no-flag-no-refresh with an injected
clock; counterfactual confirms the pacing test fails without the gate.

Follow-up to b27bdd6 on the held PR #148 branch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MaxGhenis and others added 2 commits July 21, 2026 20:19
…ement, persist rotated cookies

The reconnect watchdog's cookie refresh hard-required a
messages.google.com:OSID Chrome cookie. OSID is a per-subdomain Google
service cookie that only exists if the user has opened Messages-for-web
in that Chrome profile — on profiles that never did, every credential
repair failed with "missing required cookies: messages.google.com:OSID"
and the app looped in needs_repair forever; re-pairs bought minutes.

Proven live (2026-07-20, no re-pair): with only fresh .google.com
account cookies (SID/HSID/SSID/APISID/SAPISID + SAPISIDHASH), both
/web/config and the RegisterRefresh RPC return 200 and mint fresh 24h
tachyon tokens; a refreshed snapshot stays valid ~14 minutes while
Chrome concurrently advances the account session, then dies with
SESSION_COOKIE_INVALID — so durable health is the watchdog heal cycle,
which this unblocks.

- googlecookies: require only the five .google.com account cookies;
  carry messages.google.com OSID when present (preferred, never
  required); deterministic host tie-break via ORDER BY host_key, name
- scripts/refresh-google-session-cookies-{macos,linux}.py: mirror the
  same required/preferred split and ordering
- client/events: persist rotated cookies to session.json (sha256
  change detection, 5-min throttle, injectable clock) so restarts
  resume from fresh values instead of pair-time snapshots; take
  AuthData.CookiesLock around session marshals
- runbook: correct the self-healing section (requirements, ~14-min
  heal-cycle steady state, revive-don't-re-pair guidance)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adversarial review of the OSID self-heal fix flagged the one real
operational risk: googleCredentialRepairer.RepairCredentials has no rate
limit, and the supervisor's 5-same-fingerprint breaker resets on any
successful activity (resetFailureHistory on reaching Online). So if
Google ever revokes the exported cookie session on a short (~minutes)
cadence, connect -> brief activity -> 401 -> repair -> reconnect could
churn indefinitely and risk account-level over-connect throttling.

Add a cooldown that paces automatic repairs to >= a minimum interval
(default 90s, override via OPENMESSAGE_REPAIR_MIN_INTERVAL). The wait
runs on the parent context BEFORE the 20s refresh-timeout context, so a
too-soon repair is delayed rather than hard-parked (parking would strand
the session: Blocked is terminal with no auto-retry). Transparent to
legitimate expiry, which is measured at >= ~14 min apart. A pacedCount
(exposed via PacedRepairCount) increments on each delayed repair and
doubles as the "fast-revocation is actually happening" signal for the
durability watch. ctx cancellation during the wait returns without
flagging repair or refreshing (it's a shutdown, not a failure).

Scope: cmd/google_supervisor.go only; internal/bridge untouched. Tests
cover first-repair-immediate, pace-too-soon (>= interval + count),
after-interval-immediate, and cancel-no-flag-no-refresh with an injected
clock; counterfactual confirms the pacing test fails without the gate.

Follow-up to b27bdd6 on the held PR #148 branch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis force-pushed the claude/beautiful-hertz-fb2a1c branch from 0d79c3e to c548c8c Compare July 22, 2026 00:20
@MaxGhenis
MaxGhenis merged commit 8edb16e into main Jul 22, 2026
5 checks passed
MaxGhenis added a commit that referenced this pull request Jul 22, 2026
…-save retry, paced-repair observability (#151)

Non-blocking polish from PR #148's cross-family review (sol). #148 made maybePersistRotatedCookies rewrite session.json every ~5 minutes, which turned two latent nits into real exposure:

- internal/client/session.go: SaveSession now writes a same-dir 0600 temp file, fsyncs, and renames over session.json instead of os.WriteFile in place, so a crash mid-save can no longer truncate the only copy of the paired credentials. Mirrors googlecookies.UpdateSessionCookies, plus fsync. Regression test proven red against the old implementation.

- internal/client/events.go: a failed rotated-cookie save retries after interval/10 (~30s) instead of waiting the full ~5-min throttle, still bounded so a persistent failure can't write per event; lastCookieHash stays untouched on failure so the retry still sees the pending rotation. Test proven red against the old advance-before-save semantics.

- Paced-repair observability: delayed repairs now log a warn (wait/since_last_repair/paced_total) and publish google.repairs_paced in /api/status and MCP get_status (omitted when 0; only the transport-owning daemon builds a repairer). Runbook documents the check and the healthy-is-zero expectation.

All touched packages pass with -race; full CI green (Go Test, Go Race, Web E2E, macOS App Build, Site Build).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MaxGhenis

Copy link
Copy Markdown
Owner Author

Correction to this PR's description (posting rather than editing, so the record stays intact). A verified-Fable calibration audit found two mechanism claims here that don't hold up:

  1. "libgm rotates cookies on every response (UpdateCookiesFromResponse)" — not accurate. UpdateCookiesFromResponse merely merges whatever Set-Cookie headers the server sends, and it is called from exactly two sites (pkg/libgm/http.go:59, client.go:351). It fires per completed HTTP response (RPCs and long-poll re-opens), not continuously during an idle long-poll stream. In-band rotation is therefore not self-sustaining; the durability actually comes from the Chrome re-import path.

  2. "durable health = the watchdog heal cycle … ~every 15 min" — not borne out. That extrapolated a single out-of-band probe measurement into a steady-state law. Live data showed one cookie expiry in a 15h watch, not a 15-minute cycle.

Neither correction affects the fix itself: the root cause (an impossible messages.google.com:OSID requirement failing every repair before any network call) was re-verified against the pre-fix code, and the live evidence for the fix re-derived cleanly from the raw watch data.

Separately, credit where due: the earlier analysis in #108 had already independently identified all three components that shipped here — the OSID-absence lead, the cookie-persistence gap (with file:line cites that were more accurate than this PR's), and the missing rate limit on the repair path. The disagreement was only over whether client-side durability was achievable at all.

Runbook follow-up in #153.

@MaxGhenis

Copy link
Copy Markdown
Owner Author

Correction to my comment above: the 2026-07-25 stale-build outage lasted ~6h26m (06:54→13:21 local), not ~10.5h — the larger figure mixed UTC transcript timestamps with local watch timestamps. The mechanism correction stands unchanged.

MaxGhenis added a commit that referenced this pull request Aug 12, 2026
Every OpenMessage .app on a dev machine declared the same
CFBundleIdentifier (com.openmessage.app), so LaunchServices could resolve
"OpenMessage" (Spotlight, Dock, `open -a OpenMessage`, notification
clicks) to any of them. This caused two live outages; on 2026-07-25 a
build predating the Google self-heal OSID fix (#148) launched instead of
/Applications and latched Google Messages in needs_repair for ~10.5h
(06:54 to ~17:20).

Make it structurally impossible for a build output to win that
resolution: plain `./macos/build.sh` now stamps com.openmessage.app.dev
and emits OpenMessage-dev.dmg, while shippable builds opt in with
RELEASE=1. The build also unregisters its own output from
LaunchServices, though that alone is not durable (see below) — the dev
bundle id is what actually closes the hole.

The data dir is deliberately unaffected: BackendManager hardcodes
~/Library/Application Support/OpenMessage, so a dev build still reads
the same store. What IS bundle-id-scoped is UserDefaults (the
`defaults write com.openmessage.app V2Primary` lever), the notification
grant, and the sandbox container — so the dev-build output refuses to
suggest installing without RELEASE=1.

Runbook gains a "Bundle-id shadowing" section recording two fixes that
were verified NOT to work, so they don't get retried:

- `lsregister -u <path>` is not durable. Any LaunchServices rescan
  re-registers the bundle; a forced rescan brought all 14 back at once.
- Renaming Foo.app to Foo.app.disabled does nothing. LaunchServices
  registers on bundle structure, not the .app extension, and
  re-registered every renamed bundle at its new path.

What works for bundles kept on disk is renaming Contents/Info.plist to
Contents/Info.plist.disabled — lossless, reversible, survives forced
rescans. Also documents that mdfind is an unreliable audit here (stale
metadata, skips dot-directories) and that nested .claude/worktrees/*
builds need GOWORK=off.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MaxGhenis added a commit that referenced this pull request Aug 19, 2026
)

Every OpenMessage .app declared CFBundleIdentifier com.openmessage.app, so
LaunchServices could resolve "OpenMessage" to any stale build output — twice
causing live outages (most recently 2026-07-25: a build predating the #148
OSID fix latched Google Messages in needs_repair for ~10.5h).

Plain ./macos/build.sh now stamps a full dev identity — id
com.openmessage.app.dev, name "OpenMessage (dev)" — and emits
OpenMessage-dev.dmg, so a dev build can win neither id-based resolution
(notification clicks, open -b) nor name-based resolution (open -a
OpenMessage, Spotlight). RELEASE=1 restores the shippable identity;
release.yml sets it and asserts the built id+name before notarization, and
test.yml asserts both directions plus that stale cross-mode DMGs get
cleaned. Runbook documents the shadowing mechanics, the audit command, the
fixes that verifiably do NOT work (lsregister -u, .app renames), and the
dev-GUI sharp edge (tracked in #165).

Cross-family reviewed: sol's initial DO-NOT-MERGE (name-based resolution
hole, stale-DMG footgun) drove the CFBundleName stamp; re-review returned
MERGE with no findings. Verified on a real build: the dev bundle's LS
record registers as "OpenMessage (dev)"/com.openmessage.app.dev, and the
only record named "OpenMessage" or claiming com.openmessage.app is
/Applications/OpenMessage.app.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
mwhobrey pushed a commit to mwhobrey/om-tui that referenced this pull request Aug 24, 2026
…axGhenis#154)

Every OpenMessage .app declared CFBundleIdentifier com.openmessage.app, so
LaunchServices could resolve "OpenMessage" to any stale build output — twice
causing live outages (most recently 2026-07-25: a build predating the MaxGhenis#148
OSID fix latched Google Messages in needs_repair for ~10.5h).

Plain ./macos/build.sh now stamps a full dev identity — id
com.openmessage.app.dev, name "OpenMessage (dev)" — and emits
OpenMessage-dev.dmg, so a dev build can win neither id-based resolution
(notification clicks, open -b) nor name-based resolution (open -a
OpenMessage, Spotlight). RELEASE=1 restores the shippable identity;
release.yml sets it and asserts the built id+name before notarization, and
test.yml asserts both directions plus that stale cross-mode DMGs get
cleaned. Runbook documents the shadowing mechanics, the audit command, the
fixes that verifiably do NOT work (lsregister -u, .app renames), and the
dev-GUI sharp edge (tracked in MaxGhenis#165).

Cross-family reviewed: sol's initial DO-NOT-MERGE (name-based resolution
hole, stale-DMG footgun) drove the CFBundleName stamp; re-review returned
MERGE with no findings. Verified on a real build: the dev bundle's LS
record registers as "OpenMessage (dev)"/com.openmessage.app.dev, and the
only record named "OpenMessage" or claiming com.openmessage.app is
/Applications/OpenMessage.app.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

(cherry picked from commit 12398ce)
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