Skip to content

Fix: schedule idle prune so cached dashboard WebViews are evicted - #1386

Merged
steipete merged 2 commits into
steipete:mainfrom
naoterumaker:fix-webview-idle-prune
Jun 10, 2026
Merged

Fix: schedule idle prune so cached dashboard WebViews are evicted#1386
steipete merged 2 commits into
steipete:mainfrom
naoterumaker:fix-webview-idle-prune

Conversation

@naoterumaker

@naoterumaker naoterumaker commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Problem

OpenAIDashboardWebViewCache intends to keep the offscreen dashboard WebView alive for only 60s (idleTimeout, with a comment noting long-lived hidden ChatGPT tabs are expensive). But prune() is only invoked on the next acquire/release or preserved-page expiry — there is no timer that fires after the idle window. With the default hourly refresh cadence, the WebView released after a scrape is never pruned until the next refresh acquires it again.

Net effect: the WebKit helper processes stay resident essentially permanently. Measured on an M-series MacBook (CodexBar 0.32.6, Codex provider with web usage source): after each refresh, a com.apple.WebKit.WebContent process (~500MB RSS) plus GPU and Networking helpers (~75MB) survive the full hour between refreshes, instead of 60 seconds.

Fix

Schedule pruning for the earliest nonbusy cache entry's idle deadline. Each timer reschedules after pruning so independently released entries retain their own deadlines, and a generation guard prevents canceled/stale work items from mutating current timer state.

The idle timeout is injected through the cache initializer for focused tests. Regression coverage verifies eviction without later cache activity and verifies that a later release does not postpone an older idle entry.

Testing

  • swift test --filter OpenAIDashboardWebViewCacheTests - 17 tests passed.
  • make check - SwiftFormat and strict SwiftLint clean.
  • swift test - 3,432 tests in 394 suites passed.
  • Runtime proof used an isolated nonpersistent WKWebsiteDataStore loading about:blank, without provider auth, cookies, or Keychain access. New WebContent/GPU/Networking helper PIDs appeared; WebContent exited about 2 seconds after the test idle prune, and GPU/Networking exited about 8 seconds after appearance while the harness process remained alive.
  • autoreview --mode branch --base origin/main - clean, confidence 0.94.

🤖 Generated with Claude Code

@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 10, 2026, 7:30 AM ET / 11:30 UTC.

Summary
The PR adds autonomous earliest-deadline eviction for idle OpenAI dashboard WebViews, makes the timeout injectable, adds two regression tests, and updates the release notes.

Reproducibility: yes. Current source has no timer after release, and the isolated runtime harness provides high-confidence process-level evidence that scheduled eviction resolves the retained helper processes.

Review metrics: 2 noteworthy metrics.

  • Patch surface: 3 files; +118/-6. The resource-lifecycle fix is localized to one cache implementation, its focused tests, and one release-note entry.
  • Regression coverage: 2 tests added. The new cases cover autonomous eviction and independent deadlines for multiple cached entries.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Next step before merge

  • No automated repair is needed; maintainers can proceed with normal merge review and required checks for the current head.

Security
Cleared: The diff changes local main-actor cache scheduling, focused tests, and release notes without adding dependencies, external execution, credential access, or broader permissions.

Review details

Best possible solution:

Merge the generation-guarded earliest-deadline scheduler after required checks, retaining the autonomous and multi-entry deadline regression coverage.

Do we have a high-confidence way to reproduce the issue?

Yes. Current source has no timer after release, and the isolated runtime harness provides high-confidence process-level evidence that scheduled eviction resolves the retained helper processes.

Is this the best way to solve the issue?

Yes. Scheduling one main-actor timer for the earliest idle deadline directly enforces the existing timeout contract without introducing per-entry timer complexity or postponing older entries.

AGENTS.md: found and applied where relevant.

Codex review notes: reasoning high; reviewed against 7c0ed036e2e6.

Label changes

Label changes:

  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P2: The PR fixes substantial idle WebKit memory retention in the OpenAI web usage path, but its blast radius is provider-specific and does not make the core app unusable.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): Inspectable after-fix live output shows WebContent, GPU, and Networking helpers terminating while the isolated nonpersistent WebView harness remains alive.
  • proof: sufficient: Contributor real behavior proof is sufficient. Inspectable after-fix live output shows WebContent, GPU, and Networking helpers terminating while the isolated nonpersistent WebView harness remains alive.
Evidence reviewed

What I checked:

Likely related people:

  • Peter Steinberger: Current-main blame attributes the cache implementation and testing seams to his v0.32.5 release commit, and he authored the PR's scheduler-hardening commit after reviewing the original implementation. (role: introduced behavior and recent area contributor; confidence: high; commits: 920997c6a365, 779c4d1dda7c; files: Sources/CodexBarCore/OpenAIWeb/OpenAIDashboardWebViewCache.swift, Tests/CodexBarTests/OpenAIDashboardWebViewCacheTests.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a05be04f48

ℹ️ 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".

/// which can be an hour away on slow refresh cadences — leaving the hidden WebKit
/// helper processes (WebContent/GPU/Networking) resident the whole time.
private func scheduleIdlePrune() {
self.idlePruneWorkItem?.cancel()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep the earlier idle-prune timer armed

When multiple dashboard caches are released within the idle window, canceling the single global work item here lets a later release postpone eviction of an older idle entry. For example, if store A is released at t=0 and store B at t=50s, the t=61s prune for A is canceled and the next no-activity prune is scheduled for about t=111s, so A's WebView/WebKit helpers survive almost another minute past the intended 60s timeout. Please schedule against the earliest pending expiration or keep per-entry timers so older entries are still pruned on time.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 10, 2026
Naoteru Nakamura and others added 2 commits June 10, 2026 12:03
The webview cache evicts entries idle longer than 60s, but prune() only
ran on the next acquire/release. With an hourly refresh cadence the
hidden ChatGPT WebView - and its WebContent (~500MB), GPU and Networking
helper processes - stayed resident for the whole hour, effectively
permanently. Schedule a prune after each release so eviction happens at
the idle timeout the comment already promises.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@steipete
steipete force-pushed the fix-webview-idle-prune branch from a05be04 to 779c4d1 Compare June 10, 2026 11:06
@steipete

Copy link
Copy Markdown
Owner

Rebased onto current main, hardened the scheduler for multiple idle deadlines and stale canceled timers, and added focused regression coverage.

Runtime proof used an isolated nonpersistent about:blank WebView without provider auth, cookies, or Keychain access. WebContent exited after the idle prune; GPU and Networking helpers also exited while the harness process remained alive.

Local proof: 17 focused tests, make check, full 3,432-test suite, and branch autoreview clean at 0.94 confidence.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. P2 Normal priority bug or improvement with limited blast radius. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 10, 2026
@steipete

Copy link
Copy Markdown
Owner

Inspectable runtime proof from the isolated nonpersistent about:blank WebView harness:

20s appeared: 27116  17360 /System/Library/Frameworks/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.GPU.xpc/Contents/MacOS/com.apple.WebKit.GPU
20s appeared: 27117  13728 /System/Library/Frameworks/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.Networking.xpc/Contents/MacOS/com.apple.WebKit.Networking
20s appeared: 27118  30192 /System/Library/Frameworks/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.xpc/Contents/MacOS/com.apple.WebKit.WebContent
22s exited: 27118
28s exited: 27116
28s exited: 27117

The harness process remained alive through 30 seconds. The injected cache timeout was 0.2 seconds; WebKit performed its own delayed helper teardown after cache eviction. No provider auth, browser cookies, or Keychain access was used.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 10, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 10, 2026
@steipete
steipete merged commit 1246ec6 into steipete:main Jun 10, 2026
4 checks passed
@steipete

Copy link
Copy Markdown
Owner

Landed in 1246ec6.

Proof:

  • 17 focused cache tests passed.
  • make check passed.
  • Full local suite passed: 3,432 tests in 394 suites.
  • Branch autoreview clean at 0.94 confidence.
  • Runtime trace showed WebContent, GPU, and Networking helpers exit after idle eviction while the harness remained alive.
  • GitHub CI green: macOS, Linux x64, Linux arm64, and GitGuardian.

No provider auth, browser cookies, or Keychain access was used for runtime validation.

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

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants