Make Augment keepalive stop idempotent#1262
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 1, 2026, 4:16 AM ET / 08:16 UTC. Summary Reproducibility: yes. Source inspection shows current main can call Augment stop while disabled and log a stopped message without an active keepalive; the PR body also includes negative test output against main. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the nil-keepalive guard with the focused regression test after draft/check completion and any required make check validation are confirmed. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main can call Augment stop while disabled and log a stopped message without an active keepalive; the PR body also includes negative test output against main. Is this the best way to solve the issue? Yes. Returning early only when no keepalive exists is the narrowest maintainable fix, and the added test covers the repeated disabled stop plus settingsDidChange path. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 4756ba06bf42. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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 re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
Summary
Skip Augment keepalive stop work when no keepalive is active, avoiding repeated stop logs for already-stopped runtimes.
Context
Part of the UI hang/lag cleanup set: this removes redundant disabled-provider runtime work and log churn from repeated stop paths.
Validation
swift test --filter AugmentProviderRuntimeTestsswift test --filter AugmentStatusProbeTestsgit diff --checkRuntime Proof
Fixed-branch test proof at
d0b2082d:Negative proof against
mainusing a temporary worktree with only the new test file applied:Live app runtime proof from a freshly packaged debug app at
d0b2082d, captured June 1, 2026 at 00:12 PDT with unified logging. App startup exercises the disabled-provider runtime path (stopfollowed bysettingsDidChange). The log is redacted to startup/provider state only:Compatibility check already run for this PR:
Also ran
git diff --check; it passed with no output. The runtime proof does not include account, token, cookie, or host data.