This repository was archived by the owner on Jun 8, 2026. It is now read-only.
FEAT: persist symphony loops on reboot - #64
Merged
Merged
Conversation
- isolate plugin-discovery tests from the developer HOME so they cannot spawn real cached wrappers - lock setupStubClaude to the fake PATH so evaluate tests cannot resolve the real Claude CLI from a login shell - tighten SSRF loop tests to use isolated allowlists and non-existent repos for deterministic early exits Testing - pnpm exec tsx --test test/symphony-loop-evaluate-code.test.ts test/symphony-loop-evaluate-plan.test.ts test/symphony-loop-ssrf.test.ts test/gateway-server.test.ts - pnpm exec tsx --test test/*.test.ts Risks - plugin-discovery tests now intentionally assert the no-plugin-cache path and rely on separate fake-plugin tests for spawn coverage Made-with: Cursor
- Add BootRecoveryService to finalize dead RUNNING jobs after reconcile - Extend LocalJob with offsets, apiBaseUrl, and finalization timestamps - Extract loop-finalizer, diagnostics-helpers, and token-usage for reuse - Route normal completion through finalizeLoopFromRuntime when JobStore exists - Persist runtime.json and execution-result.json for recovery replay - Output tailer optional onOffset for lastObservedJsonlOffset updates - Add job.recovery.finalize_replayed telemetry and TelemetryEmitter type - Add boot-recovery and loop-finalizer unit tests Testing: not run in this session; run `just desktop-test` before merge Risks: Duplicate completed events if API lacks idempotency; mitigated by persisted finalization markers Made-with: Cursor
aponamarev
marked this pull request as draft
March 31, 2026 16:17
- persist runner tokens for live loops so recovered loops can resume posting output after restart - backfill recovered job paths and add recovery logging so output tailers and finalization are observable - stop silent output tailer auth failures and cover recovery auth behavior with tests Testing: - just test - just desktop-typecheck Risks: - runner token refresh still depends on a cloud endpoint that currently returns 404, so recovery falls back to the persisted token Made-with: Cursor
- Remove refreshRunnerToken and the POST to /loops/:id/runner-token (404 on prod) - Resolve loop auth synchronously from persisted file with API key fallback - Update boot-recovery tests: persist token for dead-job case, expect loop-token Testing: tsx --test test/boot-recovery.test.ts; just desktop-typecheck Risks: None identified; reattach still uses on-disk runner JWT until it expires Made-with: Cursor
- Route jobStore loop completion through finalizeLoopFromRuntime only for artifact upload and completed events; legacy no-jobStore path unchanged - Remove unused assertPathAllowed from LoopFinalizerDeps and call sites - Drop runtime.json write and pidStartedAt from LocalJob / spawn upsert - Update loop-finalizer tests for trimmed finalizer deps Testing: pnpm exec tsc -p apps/desktop/tsconfig.json --noEmit; node --test boot-recovery.test.ts loop-finalizer.test.ts; symphony-loop-execute, symphony-loop-plan, symphony-loop-cloud-failures Risks: Finalizer-owned uploads use slimmer metadata than the old pre-finalizer path; parity with API expectations should be monitored Made-with: Cursor
- Add LoopTokenStore (electron-store + safeStorage) keyed by loopId - Wire Desktop app, gateway server, boot recovery, live symphony path - Remove token from store after successful upload and completed event - Legacy workdir token file still read when store miss; tests use stub crypto Testing: pnpm exec tsc -p tsconfig.json --noEmit && pnpm test (apps/desktop) Risks: Token persistence requires Electron safeStorage in production; partial finalization paths intentionally retain token for retry Made-with: Cursor
- branch recovery finalization so only successful statuses upload artifacts and emit completed events - preserve FAILED/CANCELLED/STOPPED/UNKNOWN states during replay and emit status-appropriate error telemetry/events - expand boot-recovery and loop-finalizer coverage for non-success recovery outcomes Testing: - pnpm -C apps/desktop exec tsx --test test/loop-finalizer.test.ts test/boot-recovery.test.ts Risks: - recovery now posts error events for STOPPED/UNKNOWN outcomes, which may affect downstream loop state handling expectations Made-with: Cursor
- Fail closed during boot recovery by requiring per-loop tokens from LoopTokenStore. - Remove legacy workdir token persistence and add coverage for skipping dead/live loops when tokens are missing. Testing: - pnpm test -- test/boot-recovery.test.ts - pnpm typecheck Risks: - Loops without a persisted token now skip recovery network calls and require operator visibility via warnings. Made-with: Cursor
- Dropped backfillJobPaths; reattach uses job paths set at spawn - Removed legacy test for jobs without persisted jsonlPath Testing: apps/desktop pnpm typecheck, lint, and test Risks: Legacy JobStore rows missing jsonlPath no longer get derived paths on reattach; tailer stays off for those rows Made-with: Cursor
aponamarev
marked this pull request as ready for review
March 31, 2026 19:58
- Map CANCEL_PENDING to CANCELLED in finalizeLoopFromRuntime after live-PID guard - Persist CANCEL_PENDING as CANCELLED in persistFinalJobStatus when not success - Add loop-finalizer and boot-recovery regression tests Testing: pnpm -C apps/desktop test test/loop-finalizer.test.ts test/boot-recovery.test.ts Risks: Low; aligns persisted status with JobStore TERMINAL_STATUSES Made-with: Cursor
- Advance onOffset only after newline-framed bytes and 2xx output POST when required; skip advance for partial lines and 401/403 - Retry framed output on throttle or non-2xx POST instead of dropping bytes - Document lastObservedJsonlOffset as replay-safe in job-store and tailer calls - Add tests for partial JSONL, 403 output, and boot recovery incomplete tail Testing: apps/desktop: tsx --test symphony-loop-output-events boot-recovery; pnpm run typecheck; pnpm run lint Risks: Persisted offset can lag file EOF until a frame is committed Made-with: Cursor
- Split BootRecoveryService into reattachLiveJobs and startDeadJobFinalization; keep live reattach before gateway bind - Run dead-loop finalization after server.start with logged failures - Track background tasks; quiesce briefly on shutdown after dispose Testing: just desktop-test; just desktop-typecheck Risks: Brief window after bind before dead jobs finalized; shutdown may leave one in-flight finalization running up to quiesce timeout Made-with: Cursor
…ry that never ran (terminal COMPLETED jobs are not reconciled as active) - Always call deleteLoopToken after persistFinalJobStatus when loopTokenStore is set; early returns unchanged - Add regression test for failed artifact upload - Bump desktop to 0.9.14 Testing: pnpm test (apps/desktop) Risks: Failed artifact or completed-event POST no longer leaves a token for a hypothetical retry; uploads/events are not auto-retried on next boot
- Read CLOSEDLOOP_WATCHER_POLL_MS at setInterval creation (default 3000) - Tests: fast poll in beforeEach; shorter wait vs sleep(3400); restore env - Document variable in apps/desktop/README.md Testing: not re-run in this commit Risks: Low; production default unchanged Made-with: Cursor
thadeusb
reviewed
Apr 1, 2026
thadeusb
reviewed
Apr 1, 2026
thadeusb
reviewed
Apr 1, 2026
thadeusb
approved these changes
Apr 1, 2026
shafty023
reviewed
Apr 1, 2026
shafty023
reviewed
Apr 1, 2026
shafty023
reviewed
Apr 1, 2026
- Add cloudFinalizedAt, recoveryAttempts, lastRecoveryError on LocalJob - Refactor finalizeLoopFromRuntime outcomes; gate token delete on retry policy - Boot recovery merges dead + terminal candidates; cap recovery at 3 attempts - Preserve live-exit warning merge ordering; extend boot/loop-finalizer tests Testing: pnpm test -- test/boot-recovery.test.ts test/loop-finalizer.test.ts; pnpm typecheck in apps/desktop Risks: Token retained longer on retryable cloud failures until success or cap Made-with: Cursor
- Replace permanent authFailed stop with bounded exponential backoff for 401/403, 5xx, and network errors (env-tunable base/max delay and max attempts) - Keep replay-safe offsets; reset retry state on successful 2xx output POST - Let flush() bypass backoff for a final delivery attempt - Extend event test server with per-request output status sequences; add tests for retry recovery and flush bypass Testing: pnpm exec tsx --test test/symphony-loop-output-events.test.ts (apps/desktop) Risks: After max retries the tailer stops posting output until process restart; backoff increases API load briefly during outages Made-with: Cursor
- Delete persisted loop tokens from handleProcessCompletion on non-zero exit, cancellation early-returns, NO_WORK_PRODUCED, and legacy completion path - Clear tokens in handleLoopRequest finally when spawn never succeeds - Add LoopTokenStore.listLoopIds and BootRecoveryService.sweepOrphanedTokens after finalizeDeadJobs/reattachLiveJobs - Add unit and integration tests; bump desktop to 0.10.3 Testing: just desktop-typecheck, just desktop-lint, tsx --test on affected test files Risks: sweep removes tokens for unknown loop IDs; recovery still needs token for jobs not yet cloud-finalized (sweep keeps those) Made-with: Cursor
- Add sessionId from session-id.txt and branchName fallback from worktree git - Mirror fields in artifact upload metadata for parity with legacy desktop path - Add loop-finalizer regression tests (session, branch, EXECUTE precedence, upload) - Set desktop package version to 0.10.0 Testing: pnpm test -- test/loop-finalizer.test.ts Risks: None identified; cloud consumers receive prior correlation fields again Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symphony loops now survive an Electron restart in a useful way: if a loop’s Claude child process is still running, the desktop app re-registers it with the gateway, resumes NDJSON output tailing from the last persisted offset, and watches the PID so completion still triggers API finalization. Loops that died while the app was down (or never had a final status posted) are finalized on the next boot when API credentials are available, using artifacts and logs read from disk so the cloud loop record can close cleanly.
Key Changes
Core Feature:
BootRecoveryServicewired fromDesktopApplicationafterreconcileJobStore(): finalizes unfinalized dead jobs (finalStatusPersistedAt), reattaches live PIDs viaregisterRecoveredLoop, restartsstartOutputTailerwithlastObservedJsonlOffset, and finalizes when the watcher sees the process exit.loop-finalizer.tsto post loop completion events to${apiBaseUrl}/loops/${loopId}/eventswith command-specific artifact payloads (plan, judges, execution results, PRD content, etc.), log/tail diagnostics, token usage parsing, and job-store updates includingfinalStatusPersistedAt.LocalJob/JobStoreand refactoredsymphony-loop.tsto share recovery and finalization paths with the main loop lifecycle.Supporting Changes:
output-tailer/ gateway test adjustments; desktoppackage.jsonpatch version bump.