Fix Google self-heal: drop impossible messages.google.com:OSID requirement, persist rotated cookies - #148
Conversation
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>
…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>
0d79c3e to
c548c8c
Compare
…-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>
|
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:
Neither correction affects the fix itself: the root cause (an impossible 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. |
|
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. |
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>
) 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)
…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)
The bug (live install, reproduced 4×)
A fresh Google re-pair connects, holds ~3-4 minutes, then flips to
auth_expired/needs_repairwith "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)
internal/googlecookies.LoadChromeCookies) hard-required amessages.google.com:OSIDChrome 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 (zeromessages.google.comrows), every repair failed withmissing required cookies: messages.google.com:OSID→FlagGoogleNeedsRepair→ backoff → retry → forever. Same impossible requirement in both refresh scripts.UpdateCookiesFromResponse) but the app persisted them only onAuthTokenRefreshed(~daily), so each generation/restart resumed from pair-time cookies.Live evidence (no re-pair, probe against a copy of the real session)
.google.comaccount cookies (SID/HSID/SSID/APISID/SAPISID + SAPISIDHASH),/web/configand theRegisterRefreshRPC return 200 and mint fresh 24h tachyon tokens — the "dead" session revived instantly.SESSION_COOKIE_INVALIDwhile 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.Changes
internal/googlecookies: require only the five.google.comaccount cookies; carrymessages.google.comOSID 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);CookiesLockheld 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 scriptspy_compile✅requiredCookieschange makesTestLoadChromeCookiesDoesNotRequireMessagesOSIDfail; restoring passes./api/statuswatch: 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