Repository navigation
fix(projects): a failed project import waits a backoff, not the next chat index change (v2.2.10 P0) - #28622
fix(projects): a failed project import waits a backoff, not the next chat index change (v2.2.10 P0)#28622lawrencecchen wants to merge 8 commits into
Conversation
… index change v2.2.10-opt P0 (Lawrence, "I can't even make new workspaces"): 5,311 "project import failed" in 17 h on a Mac at 19.9 of 21.5 GB swap. A failed project.sync leaves the next relay fresh, so every chat index change sends project.sync again (the daemon rereads every editor source) and every harness's list again, with no wait. ProjectsImportRetryTests counts the import's attempts on a fake daemon target whose sends fail, with a manual clock: after a failure, 20 index changes must add no attempt, the retry comes once at the end of its wait, and a new daemon connection retries at once. This commit adds only the seams the test needs (Target and Sent, the injected clock, indexChanged, inFlight, pendingRetry); behavior is unchanged, so the first and third tests fail here (21 attempts after 20 changes; no pending retry).
…chat index change Root cause of the v2.2.10-opt P0 loop (5,311 "project import failed" in 17 h): ProjectsImport relayed on every ChatsFeed publish, and a failure fed the next attempt. A failed project.sync left connectionID unset, so the next relay was fresh again: it cleared what was sent, sent project.sync (the daemon rereads every VS Code, Cursor and Zed source) and every harness's complete list again, each with a new idempotency key. Every agent turn changes a chat, so under load (2 s request limit, swap, a daemon still running the timed-out sync) each change started one more full import. Now a failed relay waits Backoff (2 s, doubling to 5 min, jittered; CmuxNextWakeups) before it runs again. Index changes during the wait ride on that one retry, which reads the index as it is when it runs. A new daemon connection is an event that can make the import succeed: it cancels the wait and relays at once. A success resets the backoff. After N failures the attempts grow with log(time), not with the number of chat changes. Regression: ProjectsImportRetryTests (CmuxNextAppTests), red on the parent commit.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base c7c9a16; does not block |
…he seams Takes back a side change of the seam commit daf63e3: it made deliver @Concurrent, so the sends ran off the main actor. That is a behavior change, and a red commit may add only the test and its seams. deliver is main-actor again, as the relay's sends ran before daf63e3. Moving the sends off main is a separate perf change that needs its own main-thread measurement. ProjectsImportRetryTests does not depend on where the sends run: the test's target records each send itself.
|
Found 10 test failures on Blacksmith runners: Failures
|
# Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextApp/Projects/ProjectsImport.swift
…pp passes its target, socket and connection The merge of feat (acpmux groups the projects, #28055) with this PR's retry backoff left CmuxNextApp 86 lines over its ratchet (arch-ratchet app_lines). ProjectsImport now knows no app type: AppServices passes the local daemon as a Target (Target(daemon:)), the acpmux socket path and the connection it follows. Behavior is unchanged; ProjectsImportRetryTests builds it with the same fake target, a nil socket and a nil connection.
…blic Target and Batch use DaemonConnection and JSONValue) Fleet compile job a1d2f943314affa5ea612f21 at 867d892 refused ProjectsImport.swift:44 and :78.
|
Review (claude-review-subagent-cw): SHIP at ae6bb77 I found no blocking defect. I read the merge, the two later commits and the net diff over feat 22abf5b. I did not compile or run tests. I used the fleet results that the resolver gave (compile 6ff02b629a0864a6594e1d99, ProjectsImportRetryTests step fa7c719fcaa0b14e1bd7fbce). Merge f9f44be (remerge-diff)
Move 867d892 and public import ae6bb77
Read failure (
Not blocking: a reconnect during a failed send waits the backoff (PR logic, not the merge)
Nits
|
|
Conflict wave: merged feat-cmux-next 22abf5b into this branch (merge f9f44be), then two commits.
Fleet evidence at ae6bb77:
I did not add |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
P0 for v2.2.10-opt ("I can't even make new workspaces"). The capture shows
project import failed5,311 times in 17 h (06:41Z to 23:52Z on 2026-10-30), on a Mac at 19.9 of 21.5 GB swap and load 13.Cause:
ProjectsImportrelayed on everyChatsFeedpublish, and a failure fed the next attempt. A failedproject.syncleftconnectionIDunset. So the next relay was fresh again: it cleared what was sent, sentproject.sync(the daemon rereads every VS Code, Cursor and Zed source) and every harness's complete list again, each with a new idempotency key. Every agent turn changes a chat, so under load each change started one more full import.Fix: a failed relay now waits a
Backoff(2 s, doubling to 5 min, jittered) before it runs again. Index changes during the wait ride on that one retry, which reads the index as it is when it runs. A new daemon connection cancels the wait and relays at once. A success resets the backoff. After N failures, the number of attempts grows with log(time), not with the number of chat changes.Commits:
Target/Sent, an injected clock,indexChanged,inFlight,pendingRetry). This commit also moved the sends off the main actor (deliverbecame@concurrent). That is a behavior change, and it must not be in a red commit. Commit 3 takes it back. I did not rewrite the pushed history.deliverruns on the main actor again, as before commit 1. Moving the sends off main is a separate perf change with its own main-thread measurement (recorded in followups).The importer now reads the chat feed that
start(feed:)receives, notservices.chatsFeed. This is a pure seam: the one caller (AppDelegate.swift:317) passesservices.chatsFeed, which is the same object. AppServices holds it for its life, so the weak reference does not change behavior.Clean red ref:
fix/p0-project-import-loop-redat 944e4ef (proof only, never merge). It is this head with the fix ac2d9a9 reverted: feat, plus the pure seams, plus the test, with no backoff and no off-main change.Changelog
Fixed: a project import that fails no longer runs again on every chat change. It waits longer after each failure.
Testing
ProjectsImportRetryTests(CmuxNextAppTests), with a manual clock and a count of attempts. No wall time decides a result.aFailingImportWaitsItsBackoffInsteadOfRetryingOnEveryIndexChange: after a failure, 20 index changes add no attempt, and the retry after the wait runs once.aNewDaemonConnectionRetriesAtOnceaRetryThatSucceedsEndsTheBackoffRed on 944e4ef (the clean red ref), green on 7768eea: the fleet runs are pending. This PR gets the job ids when they end.
Observed / Not run
No capture: no UI change.
Follow-ups (in followups.md): log the error type as public, so a capture says which request failed; continue past a refused list.
perf claim: none