Skip to content

Avoid duplicate Agent Sessions remote refresh passes - #2079

Merged
steipete merged 5 commits into
steipete:mainfrom
Yuxin-Qiao:fix/agent-sessions-coalesce-overlap-refresh
Jul 12, 2026
Merged

Avoid duplicate Agent Sessions remote refresh passes#2079
steipete merged 5 commits into
steipete:mainfrom
Yuxin-Qiao:fix/agent-sessions-coalesce-overlap-refresh

Conversation

@Yuxin-Qiao

@Yuxin-Qiao Yuxin-Qiao commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Coalesce unchanged overlapping Agent Sessions remote refresh requests into the active pass instead of scheduling a duplicate Tailscale discovery and SSH sweep.
  • Preserve generation invalidation, stale-result suppression, and one immediate retry when Agent Sessions settings change during a pass.
  • Add focused gate coverage and document the user-visible efficiency fix with contributor credit.

Validation

  • Exact head: 61cdbdc21983fa0902406df14a2f28c1defa636c, synchronized with current main.
  • Focused Agent Sessions/Tailscale and synchronized-main suites: 115/115 passed.
  • Settings-change gate proof: stale generation is not published and exactly one current-generation retry follows.
  • make check: generated docs, locale checks, SwiftFormat, and SwiftLint clean.
  • Zero-retry full suite: 624/624 selections across 52/52 groups, all first-pass; no retries or timeouts.
  • Autoreview: clean, no accepted/actionable findings; 0.96 correctness confidence.
  • Exact Developer ID-signed debug bundle: embedded commit 61cdbdc2; strict code-sign and Gatekeeper accepted.
  • Packaged live app with isolated config, Keychain disabled, and local no-network Tailscale/SSH wrappers: five menu opens completed in 2.028 seconds during one blocked pass; counts stayed at discovery/fetch 1/0, then settled at 1/1; the menu rendered qa-host — 1 / qa-shared.
  • Production CodexBar PID, executable path, start time, and restored debug defaults remained unchanged.

Risk

Low. Concurrent callers now share an active remote failure instead of forcing an immediate duplicate pass; the next menu open or normal 60-second cycle retries. Settings changes still force one current-generation pass.

Dependencies

No dependency changes; freshness review not applicable.

Opening the menu during an in-flight remote refresh was marking the gate pending, which forced a second Tailscale/SSH pass even when settings had not changed. Keep pending reserved for settingsDidChange so unchanged overlaps merge into the current pass.

Co-authored-by: Cursor <cursoragent@cursor.com>
@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. P2 Normal priority bug or improvement with limited blast radius. labels Jul 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 12, 2026, 12:03 AM ET / 04:03 UTC.

Summary
The PR makes unchanged overlapping Agent Sessions remote refresh requests share the active pass, adds focused gate regression tests, and records the efficiency fix in the changelog.

Reproducibility: yes. Current main deterministically marks an ordinary in-flight overlap as pending and therefore schedules a duplicate pass; the exact-head packaged-app harness demonstrates the corrected single-pass behavior across five menu opens.

Review metrics: 3 noteworthy metrics.

  • Production change: 1 line removed. The functional diff only stops ordinary overlaps from setting pending retry state.
  • Regression coverage: 4 focused tests added. The cases distinguish unchanged overlap coalescing from settings-generation invalidation and retry behavior.
  • Runtime concurrency proof: 5 menu opens, 1 discovery, 1 fetch. The exact-head packaged-app run demonstrates that overlapping UI requests share one remote pass.

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.

Risk before merge

  • [P1] Merging intentionally means callers overlapping a failing active Tailscale/SSH pass share that failure and wait for the next menu open or normal 60-second refresh cycle rather than forcing an immediate duplicate attempt.

Maintainer options:

  1. Accept shared-failure retry timing (recommended)
    Merge with the documented tradeoff that an overlapping active failure is retried on the next menu open or normal refresh cycle rather than through redundant immediate work.

Next step before merge

  • No automated repair or product decision remains; keep this maintainer-engaged PR open for normal merge handling after the required exact-head checks finish.

Security
Cleared: The diff changes only an internal refresh gate, focused tests, and release notes, with no dependency, credential, permission, download, or build-execution changes.

Review details

Best possible solution:

Merge the minimal gate correction and focused regression coverage after required exact-head checks complete, retaining the documented next-cycle retry behavior after a shared active failure.

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

Yes. Current main deterministically marks an ordinary in-flight overlap as pending and therefore schedules a duplicate pass; the exact-head packaged-app harness demonstrates the corrected single-pass behavior across five menu opens.

Is this the best way to solve the issue?

Yes. Reserving pending state for settings changes is the narrowest maintainable fix because it preserves generation invalidation, stale-result suppression, and one settings-driven retry without introducing another concurrency mechanism.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This fixes bounded redundant remote work in Agent Sessions with limited blast radius and strong validation.
  • merge-risk: 🚨 availability: The merge intentionally changes immediate retry timing for callers that overlap a failing active remote pass.
  • 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): Exact-head packaged-app evidence directly shows five overlapping menu opens sharing one blocked pass, one discovery/fetch sequence, and the expected rendered Agent Sessions result.
  • proof: sufficient: Contributor real behavior proof is sufficient. Exact-head packaged-app evidence directly shows five overlapping menu opens sharing one blocked pass, one discovery/fetch sequence, and the expected rendered Agent Sessions result.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Current history attributes the Agent Sessions store and refresh gate to the v0.42.0 release commit, and the repository owner performed repeated focused, full-suite, signing, and live-bundle validation on this PR's exact heads. (role: feature introducer and recent validator; confidence: high; commits: 7ddcf28b0442, 1bc90bdcdc62, 931682fe8272; files: Sources/CodexBar/AgentSessionsStore.swift, Tests/CodexBarTests/AgentSessionMenuDescriptorTests.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.
Review history (5 earlier review cycles)
  • reviewed 2026-07-12T01:50:45.593Z sha 46d0e65 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-12T02:51:42.122Z sha 1bc90bd :: needs maintainer review before merge. :: none
  • reviewed 2026-07-12T02:56:07.499Z sha 1bc90bd :: needs maintainer review before merge. :: none
  • reviewed 2026-07-12T03:42:43.371Z sha 931682f :: needs maintainer review before merge. :: none
  • reviewed 2026-07-12T03:48:30.907Z sha 931682f :: needs maintainer review before merge. :: none

@steipete

Copy link
Copy Markdown
Owner

@clawsweeper re-review

Final maintainer validation is complete on exact head 1bc90bdcdc62ace5dda86ae6176fd3e4172e3e98.

  • synchronized with current main while preserving @Yuxin-Qiao's original commit and credit
  • unchanged overlaps now coalesce; settings changes still invalidate the stale generation and request exactly one retry
  • focused Agent Sessions/Tailscale suites: 15/15 passed
  • make check: clean
  • zero-retry full suite: 614/614 selections across 52/52 groups, all first-pass; no retries or timeouts
  • autoreview: clean, no accepted/actionable findings; 0.95 correctness confidence
  • exact signed debug bundle: embedded commit 1bc90bdc; strict code-sign and Gatekeeper accepted
  • packaged live app proof: with isolated config, Keychain disabled, and local no-network wrappers, five real menu opens during one blocked pass produced discovery/fetch counts 1/1; the menu rendered the deterministic qa-host — 1 / qa-shared result
  • focused generation proof: an in-flight settings change suppresses the stale result and produces exactly two total passes with only the current result publishable
  • production app PID/path/start time and the backed-up debug defaults were unchanged after restoration
  • dependencies unchanged; freshness review not applicable

Residual risk: callers overlapping an active failure share that failure until the next menu open or normal 60-second cycle; they no longer force a redundant immediate pass.

@clawsweeper

clawsweeper Bot commented Jul 12, 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 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. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. and removed 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 Jul 12, 2026
@steipete

Copy link
Copy Markdown
Owner

@clawsweeper re-review

Final maintainer validation is complete on exact head 931682fe82721eaa30cadba13c51b4236b9603a8.

  • synchronized with current main while preserving @Yuxin-Qiao's original commit and credit
  • unchanged overlaps coalesce; settings changes still invalidate the stale generation and request exactly one retry
  • focused Agent Sessions/Tailscale and synchronized-provider suites: 73/73 passed
  • make check: clean
  • zero-retry full suite: 615/615 selections across 52/52 groups, all first-pass; no retries or timeouts
  • autoreview: clean, no accepted/actionable findings; 0.98 correctness confidence
  • exact signed debug bundle: embedded commit 931682fe; strict code-sign and Gatekeeper accepted
  • packaged live app proof: with isolated config, Keychain disabled, and local no-network wrappers, five real menu opens completed in 1.843 seconds while the first pass was blocked; discovery/fetch remained 1/0, then settled at 1/1; the menu rendered qa-host — 1 / qa-shared
  • focused generation proof: an in-flight settings change suppresses the stale result and produces exactly two total passes with only the current result publishable
  • production app PID/path/start time and the backed-up debug defaults were unchanged after restoration
  • dependencies unchanged; freshness review not applicable

Residual risk: callers overlapping an active failure share that failure until the next menu open or normal 60-second cycle; they no longer force a redundant immediate pass.

@clawsweeper

clawsweeper Bot commented Jul 12, 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:

@steipete

Copy link
Copy Markdown
Owner

@clawsweeper re-review

Final maintainer validation is complete on exact head 61cdbdc21983fa0902406df14a2f28c1defa636c.

  • synchronized with current main while preserving @Yuxin-Qiao's original commit and credit
  • unchanged overlaps coalesce; settings changes still invalidate the stale generation and request exactly one retry
  • focused Agent Sessions/Tailscale and synchronized-main suites: 115/115 passed
  • make check: clean
  • zero-retry full suite: 624/624 selections across 52/52 groups, all first-pass; no retries or timeouts
  • autoreview: clean, no accepted/actionable findings; 0.96 correctness confidence
  • exact signed debug bundle: embedded commit 61cdbdc2; strict code-sign and Gatekeeper accepted
  • packaged live app proof: with isolated config, Keychain disabled, and local no-network wrappers, five real menu opens completed in 2.028 seconds while the first pass was blocked; discovery/fetch remained 1/0, then settled at 1/1; the menu rendered qa-host — 1 / qa-shared
  • focused generation proof: an in-flight settings change suppresses the stale result and produces exactly two total passes with only the current result publishable
  • production app PID/path/start time and the backed-up debug defaults were unchanged after restoration
  • dependencies unchanged; freshness review not applicable

Residual risk: callers overlapping an active failure share that failure until the next menu open or normal 60-second cycle; they no longer force a redundant immediate pass.

@clawsweeper

clawsweeper Bot commented Jul 12, 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:

@steipete
steipete merged commit 9a6c74c into steipete:main Jul 12, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. 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