Skip to content

perf: stop dormant agent session schedulers - #2850

Merged
steipete merged 1 commit into
mainfrom
perf/agent-session-idle-schedulers
Aug 10, 2026
Merged

perf: stop dormant agent session schedulers#2850
steipete merged 1 commit into
mainfrom
perf/agent-session-idle-schedulers

Conversation

@steipete

Copy link
Copy Markdown
Owner

Summary

  • Stop Agent Sessions local and remote periodic schedulers while their monitoring features are disabled.
  • Keep the local scheduler only for enabled Agent Sessions or consented agent-aware adaptive monitoring, and keep the remote scheduler only for enabled Agent Sessions.
  • Reject stale settings-era refresh results, coalesce overlapping refreshes, and cancel all owned work on stop/deinit.
  • Follow up on the broader idle-CPU work from perf: reduce idle cost scan work #2848 while keeping this scheduler lifecycle change independently scoped.

Tests

  • make check (0 violations)
  • release build
  • make test (839 selections, 70/70 groups, no retries or timeouts)
  • Agent Sessions scheduler suite (6/6)
  • existing Agent Sessions tests (15/15)
  • adaptive performance suite (three isolated reruns passed; an earlier parallel-loaded closeout run exceeded the existing 250 ms wall-clock gate, then passed unloaded)

@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. 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. labels Aug 10, 2026
@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 10, 2026, 3:29 PM ET / 19:29 UTC.

ClawSweeper review

What this changes

The PR stops Agent Sessions’ local and remote periodic refresh loops when their respective monitoring features are disabled, while adding lifecycle, overlap, and stale-result safeguards.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open: current main still starts both Agent Sessions schedulers unconditionally, while this PR adds scoped lifecycle control and regression coverage. Likely related people: steipete (high confidence) from the feature’s current-main and proposed lifecycle history.

Priority: P2
Reviewed head: 8584ced7003ac779e87f9e8294e0ac66eae2cbb6

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The patch is focused, preserves the local-versus-remote monitoring boundary, and includes targeted lifecycle regression coverage.
Proof confidence 🌊 off-meta tidepool Not applicable: This maintainer-authored internal scheduler change is exempt from the external contributor proof gate; its focused tests and stated full-suite run are supplemental validation.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This maintainer-authored internal scheduler change is exempt from the external contributor proof gate; its focused tests and stated full-suite run are supplemental validation.
Evidence reviewed 6 items Current-main baseline: Current main creates both periodic tasks whenever the store starts, without checking whether Agent Sessions or adaptive monitoring is enabled; the central behavior therefore remains unimplemented on main.
Lifecycle implementation: The proposed store gates periodic work on started state and the corresponding monitoring setting, then cancels periodic and immediate tasks when no longer needed.
Focused regression coverage: The new scheduler tests cover disabled/enabled states, stale local and remote results, refresh coalescing, and late publication after stop.
Findings None None.
Security None None.

How this fits together

Agent Sessions gathers local and remote session data for the menu bar. Settings determine which refresh paths run, and refreshed results feed the session state shown to the app.

flowchart LR
    A[User settings] --> B[Agent Sessions store]
    B --> C{Monitoring enabled?}
    C -->|Yes| D[Local and remote schedulers]
    C -->|No| E[Cancel owned refresh work]
    D --> F[Session refresh results]
    E --> F
    F --> G[Menu bar session state]
Loading

Before merge

  • Complete next step (P2) - No discrete repair is needed; retain this maintainer-authored PR for normal check completion and merge review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Implementation and regression coverage production +180, tests +380, changelog +1 The sizeable scheduler rewrite is paired with focused state, cancellation, and stale-result coverage.

Technical review

Best possible solution:

Land the scoped scheduler lifecycle change after the normal PR checks complete, preserving local adaptive monitoring while leaving remote discovery behind the Agent Sessions setting.

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

Not applicable: this is a proposed scheduler lifecycle and performance patch, not an issue report requiring a reporter reproduction.

Is this the best way to solve the issue?

Yes: separating local and remote scheduler eligibility preserves consented adaptive local monitoring while preventing dormant periodic work.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d7934a5f6121.

Labels

Label changes:

  • add P2: This is a bounded background-performance and session-refresh improvement rather than an urgent user-facing outage.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This maintainer-authored internal scheduler change is exempt from the external contributor proof gate; its focused tests and stated full-suite run are supplemental validation.

Label justifications:

  • P2: This is a bounded background-performance and session-refresh improvement rather than an urgent user-facing outage.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This maintainer-authored internal scheduler change is exempt from the external contributor proof gate; its focused tests and stated full-suite run are supplemental validation.

Evidence

What I checked:

Likely related people:

  • steipete: Blame assigns the new lifecycle paths to the PR head, and current main’s surrounding Agent Sessions implementation and the related merged performance work are attributed to Peter Steinberger. (role: feature owner and recent area contributor; confidence: high; commits: 8584ced7003a, d7934a5f6121; files: Sources/CodexBar/AgentSessionsStore.swift, Sources/CodexBar/StatusItemController+AgentSessions.swift)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

@steipete
steipete merged commit 52a0921 into main Aug 10, 2026
9 checks passed
@steipete
steipete deleted the perf/agent-session-idle-schedulers branch August 10, 2026 19:54
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. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant