Skip to content

feat: full FC state snapshots — dump-based store and restore#431

Merged
eddycek merged 2 commits intomainfrom
feat/snapshot-full-dump-restore
Apr 11, 2026
Merged

feat: full FC state snapshots — dump-based store and restore#431
eddycek merged 2 commits intomainfrom
feat/snapshot-full-dump-restore

Conversation

@eddycek
Copy link
Copy Markdown
Owner

@eddycek eddycek commented Apr 11, 2026

Summary

  • Snapshots now capture complete FC configuration via CLI dump (all settings including BF defaults) alongside diff (for comparison UI)
  • Restore uses defaults nosave + dump commands for exact 1:1 FC state match — fixes issue where settings at BF default values were lost on restore
  • Backward compatible: old snapshots without cliDump fall back to current diff-based restore
  • MSPClient refactored: extracted shared reboot logic into handlePostCLIExitReboot(), added exportCLIDiffAndDump() (one CLI session, one reboot)
  • 5 new tests: dump-based restore, identity filtering, backward compat, MSP PID safety net

Test plan

  • 3110 unit tests pass, 0 TS errors
  • Dump-based restore sends defaults nosave first
  • Identity commands (board_name, signature) filtered from dump
  • Legacy snapshots (no cliDump) use diff-based restore without defaults nosave
  • MSP PID restore still runs after dump-based restore
  • Manual: create snapshot → verify JSON has cliDump field
  • Manual: change PIDs → restore snapshot → verify PIDs match snapshot exactly

🤖 Generated with Claude Code

Snapshots now capture complete FC configuration via CLI `dump` (all
settings including defaults) alongside the existing `diff` (for
comparison UI). Restore uses `defaults nosave` + dump commands for
exact 1:1 FC state match.

Changes:
- MSPClient: extract handlePostCLIExitReboot() helper, add
  exportCLIDiffAndDump() combined method (one CLI session, one reboot)
- SnapshotManager: store both cliDiff and cliDump in configuration
- Restore handler: dump-based restore with `defaults nosave` when
  cliDump available, backward compat fallback to diff-based for
  legacy snapshots
- 5 new tests for dump-based restore, backward compat, MSP PID safety

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 11, 2026 13:05
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR upgrades snapshot capture/restore to support full flight-controller (FC) state restoration by storing a CLI dump (complete config) alongside the existing diff (UI comparison), and restoring via defaults nosave + dump commands for an exact 1:1 match (while remaining backward compatible with legacy snapshots).

Changes:

  • Snapshot creation now exports both CLI diff + dump in one CLI session (single reboot) and stores both in the snapshot payload.
  • Snapshot restore prefers dump-based restore (with defaults nosave) when cliDump exists; otherwise falls back to legacy diff-based restore.
  • MSPClient refactor extracts shared “exit CLI → reboot → reconnect” logic and adds exportCLIDiffAndDump(), with expanded unit/E2E coverage and docs updates.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
TESTING.md Updates test inventory counts/date.
src/main/storage/SnapshotManager.ts Stores cliDump in snapshots via combined export.
src/main/storage/SnapshotManager.test.ts Adds coverage for storing both cliDiff + cliDump.
src/main/msp/MSPClient.ts Refactors post-CLI reboot handling and adds exportCLIDiffAndDump().
src/main/ipc/handlers/snapshotHandlers.ts Restores from cliDump when present (defaults reset + dump commands), else legacy diff restore.
src/main/ipc/handlers.test.ts Adds tests for dump-based restore behavior, identity filtering, legacy fallback, and PID safety net.
src/main/e2e/tuningWorkflow.e2e.test.ts Updates E2E mocks/expectations for dump-based snapshot behavior.
src/main/demo/MockMSPClient.ts Adds demo implementation of exportCLIDiffAndDump().
src/main/CLAUDE.md Documents updated snapshot restore flow and full-state snapshots.
SPEC.md Updates test/PR counts in progress summary.
README.md Updates unit test count in documentation.
CLAUDE.md Notes snapshot dump/diff behavior and restore strategy.
ARCHITECTURE.md Updates unit test count in architecture/testing notes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +192 to +193
await deps.mspClient.connection.sendCLICommand('defaults nosave');
logger.info('Restore: sent defaults nosave (dump-based full restore)');
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In dump-based restore, defaults nosave is sent without checking/validating the CLI response. If the command is rejected or times out, the restore will continue applying commands against a non-reset state, which defeats the 1:1 restore guarantee and can produce confusing results. Consider capturing the response and running it through validateCLIResponse() (and/or treating failures as fatal for dump-based restore).

Suggested change
await deps.mspClient.connection.sendCLICommand('defaults nosave');
logger.info('Restore: sent defaults nosave (dump-based full restore)');
const defaultsResponse = await deps.mspClient.connection.sendCLICommand('defaults nosave');
validateCLIResponse('defaults nosave', defaultsResponse);
logger.info('Restore: defaults nosave validated (dump-based full restore)');

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — defaults nosave response is now validated via validateCLIResponse() in 9e28d3b.

Comment thread src/main/msp/MSPClient.ts Outdated
Comment on lines +564 to +570
try {
await this.connection.writeCLIRaw('exit');
} catch {}
await new Promise((resolve) => setTimeout(resolve, 4000));
this.connection.resetProtocol();
await this.connection.forceExitCLI();
this.connection.clearFCRebootedFromCLI();
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recoverFromCLIError() sends exit (reboot) but never sets _rebootPending=true. If the FC re-enumerates USB during this recovery reboot, the disconnected handler will clear currentPort, making auto-reconnect or subsequent reconnectAfterReboot() impossible. Consider setting _rebootPending=true before sending exit (and clearing it after recovery), or reusing handlePostCLIExitReboot() so recovery follows the same robust reboot/reconnect path as normal CLI exports.

Suggested change
try {
await this.connection.writeCLIRaw('exit');
} catch {}
await new Promise((resolve) => setTimeout(resolve, 4000));
this.connection.resetProtocol();
await this.connection.forceExitCLI();
this.connection.clearFCRebootedFromCLI();
this._rebootPending = true;
try {
try {
await this.connection.writeCLIRaw('exit');
} catch {}
await new Promise((resolve) => setTimeout(resolve, 4000));
this.connection.resetProtocol();
await this.connection.forceExitCLI();
this.connection.clearFCRebootedFromCLI();
} finally {
this._rebootPending = false;
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — recoverFromCLIError() now sets _rebootPending=true before exit and clears it in finally block. 9e28d3b.

Comment thread SPEC.md Outdated
## Progress Summary

**Last Updated:** April 9, 2026 | **Tests:** 3105 unit tests across 145 files + 37 Playwright E2E tests | **PRs Merged:** #1–#430
**Last Updated:** April 9, 2026 | **Tests:** 3110 unit tests across 145 files + 37 Playwright E2E tests | **PRs Merged:** #1–#431
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Progress Summary line updates test/PR counts, but the Last Updated date remains April 9, 2026. Since this section is being updated in this PR, the date should likely be updated too (and kept consistent with TESTING.md’s April 11, 2026 verification).

Suggested change
**Last Updated:** April 9, 2026 | **Tests:** 3110 unit tests across 145 files + 37 Playwright E2E tests | **PRs Merged:** #1#431
**Last Updated:** April 11, 2026 | **Tests:** 3110 unit tests across 145 files + 37 Playwright E2E tests | **PRs Merged:** #1#431

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — date updated to April 11 in 9e28d3b.

Comment thread ARCHITECTURE.md Outdated
# Architecture Overview

**Last Updated:** April 9, 2026 | **Phase 4 Complete, Phase 6 Complete** | **3105 unit tests, 145 files + 37 Playwright E2E tests**
**Last Updated:** April 9, 2026 | **Phase 4 Complete, Phase 6 Complete** | **3110 unit tests, 145 files + 37 Playwright E2E tests**
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Header updates the unit test count, but Last Updated still says April 9, 2026. If this doc is being maintained with each change, the date should be updated to reflect this edit (and kept consistent with other updated docs in this PR).

Suggested change
**Last Updated:** April 9, 2026 | **Phase 4 Complete, Phase 6 Complete** | **3110 unit tests, 145 files + 37 Playwright E2E tests**
**Last Updated:** April 11, 2026 | **Phase 4 Complete, Phase 6 Complete** | **3110 unit tests, 145 files + 37 Playwright E2E tests**

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — date updated to April 11 in 9e28d3b.

Address CodePilot feedback:
- Validate `defaults nosave` response via validateCLIResponse()
- Set _rebootPending=true in recoverFromCLIError() to prevent
  auto-reconnect losing currentPort during USB re-enumeration
- Update Last Updated dates to April 11 in SPEC.md and ARCHITECTURE.md

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@eddycek eddycek merged commit d062fa6 into main Apr 11, 2026
1 check passed
@eddycek eddycek deleted the feat/snapshot-full-dump-restore branch April 11, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants