Upgrade backup should include dataDir (database files not backed up)

[zylos01]

Problem

The 7-step upgrade pipeline (cli/lib/upgrade.js) creates a backup in Step 2 (step2_backup), but only backs up skillDir (~/.claude/skills/<component>/). The dataDir (~/zylos/components/<component>/) is not included in the backup.

This means database files (e.g., recruit.db), config files, and other runtime data in the component's data directory are not preserved before an upgrade.

Impact

During a Recruit ATS upgrade (v0.2.7 → v0.2.8), the migration script contained a bug that triggered CASCADE DELETE on the production SQLite database, destroying all candidate, role, evaluation, and interview data. Because dataDir was not backed up, the data was unrecoverable.

• 35 candidates lost
• 108 evaluations lost
• 28 internal interviews + 160 messages lost
• No backup available for rollback

Root Cause

In upgrade.js line 387-399, step2_backup only copies ctx.skillDir:

function step2_backup(ctx) {
  const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
  const backupDir = path.join(ctx.skillDir, '.backup', timestamp);
  copyTree(ctx.skillDir, backupDir, { excludes: ['node_modules', '.backup', '.zylos'] });
  // ...
}

ctx.dataDir is defined in createContext() (line 321) but never used in the backup step.

Proposed Fix

Add dataDir backup to step2_backup:

1. Copy dataDir contents (especially .db, .json, .sqlite files) to the backup directory alongside the skill backup
2. Include dataDir in the rollback path so failed upgrades can restore data
3. Consider backing up only data files (not logs) to keep backup size reasonable

Additional Context

• dataDir is already referenced in uninstall and info commands (component.js)
• The cleanup logic (cleanOldBackups) already retains only the latest backup, so storage impact is minimal
• Components with SQLite databases (recruit, comm-bridge, scheduler) are especially vulnerable without data backup

[voyax]

Proposed Solution: Extend preserve to cover dataDir backup

Background

PR #503 proposed backing up all top-level files in dataDir unconditionally. After discussion, we've decided on a different approach: let each component declare which files need protection, using the existing preserve field in SKILL.md.

Design

Principle: Components know best which files are important. The upgrade flow should not guess — it should follow the component's declaration.

SKILL.md (component side) — no structural change to preserve, keep the array format:

# Example: recruit-ats
preserve:
  - config.json           # no prefix → skillDir (backward compatible)
  - data://recruit.db     # data:// prefix → dataDir
  - data://config.json    # dataDir config

• No prefix → file in skillDir (~/zylos/.claude/skills/<name>/), same as today
• data:// prefix → file in dataDir (~/zylos/components/<name>/)
• Components that don't declare preserve have no files backed up from dataDir (explicit opt-in)

upgrade.js (upgrade flow side) — changes in step2_backup and rollback:

step2_backup additions:

1. Read current SKILL.md preserve list
2. For each entry, resolve the target directory (skillDir or dataDir based on prefix)
3. If file exists, copy to backup (_backup/skill/ or _backup/data/)
4. File not found → skip with warning (non-blocking)

rollback additions:

1. If _backup/data/ exists, restore files to dataDir
2. mkdirSync(dataDir) before restore (handle directory-deleted edge case)

No changes to step3_smartMerge, parseSkillMd, or other modules.

Important: preserve is backup-only, not a write lock

preserve does not prevent files from being modified during upgrade. It only takes a snapshot before the upgrade starts.

The upgrade pipeline after step 2:

• step 3 (smartSync): skillDir files (including those in preserve) are updated normally — unmodified files get overwritten by the new version, modified files are merged or kept
• step 7 (start service): the new code starts and migration scripts run, modifying dataDir files (including data:// entries) as needed

On successful upgrade, all preserve files are in their latest state — skillDir files updated by smartSync, dataDir files updated by migration. The backup is not used.

On failed upgrade, rollback restores preserve files from the step 2 backup, returning both skillDir and dataDir to their pre-upgrade state.

Why this approach

1. No structural change to preserve — stays an array, fully backward compatible. Existing components (telegram, lark, browser, etc.) continue to work without modification
2. Explicit declaration — component authors add data:// entries only for files that matter. No wasted backup space on logs, temp files, or large directories
3. Minimal code change — ~40 lines in upgrade.js (step2_backup + rollback)
4. Addresses the root cause — Recruit ATS incident happened because recruit.db was not backed up. With this change, recruit-ats SKILL.md would declare data://recruit.db, and the upgrade flow would back it up before step 3

Migration

Existing components need to add data:// entries for any critical dataDir files. For example:

• recruit-ats: data://recruit.db
• telegram: data://config.json
• lark: data://config.json

Relationship to PR #503

This proposal supersedes the approach in #503 (unconditional top-level file backup). #503 should not be merged — the implementation should follow this whitelist-based design instead.

[voyax]

Supplementary: Template & Spec Updates

The implementation should also update the component template and spec to reflect the new data:// prefix convention:

1. zylos-component-template — template/SKILL.md

Update the default preserve block — remove data/ (runtime data belongs in dataDir, not skillDir) and add data:// example:

preserve:
  - config.json
  - .env
  # Add data:// entries for critical runtime data that should be backed up before upgrade
  # - data://example.db

2. zylos-component-template — COMPONENT-SPEC.md

In §3 (SKILL.md Specification), update the preserve field documentation to include:

• data:// prefix: references files in dataDir (~/zylos/components/<name>/)
• No prefix (default): references files in skillDir (~/zylos/.claude/skills/<name>/)
• Behavior: all preserve entries are backed up before upgrade in step 2; skillDir entries are additionally protected from overwrite in step 3
• Example: data://recruit.db backs up the database before upgrade, allowing migration to run normally, with rollback restoring the pre-migration state if the upgrade fails

These changes should ship together with the upgrade.js code change to keep docs and implementation in sync.

[zylos01]

Refinement: Backup Integrity, Rotation, Service Lifecycle, Scope

Building on @voyax's preserve + data:// whitelist design, @howardzhou raised four points that tighten the backup contract. Consolidating them here as a concrete proposal.

1. Rollback safety — checksum-verified backup

Problem: an incomplete/truncated backup (disk full, process killed mid-copy) could overwrite good production state on rollback.

Solution: capture-time checksum + restore-time verification.

• After step2_backup finishes copying files, write _backup/<backup-id>/manifest.sha256 containing <sha256> <relative/path> for every file under _backup/skill/ and _backup/data/
• rollback flow:
  1. Read manifest.sha256
  2. Re-hash every file in the backup; compare against the manifest
  3. On any mismatch or missing file → abort rollback, leave current state untouched, surface a loud error. Do not proceed to restore.
  4. All match → restore as designed

Not doing: dry-run restore (copy to temp location, verify, then swap). Adds non-trivial complexity for SQLite / config-file workloads with limited additional safety on top of checksum verification. Can be revisited if real-world incidents warrant it.

2. Backup directory naming — <component>_<timestamp>/

Each upgrade generates a backup under:

~/zylos/components/<component>/_backup/<component>_<YYYYMMDD-HHMMSS>/
├── skill/        # preserve entries without prefix
├── data/         # preserve entries with data:// prefix
└── manifest.sha256

Timestamp in local TZ (Asia/Singapore) for human readability. Component name in the dirname even though it's already scoped under <component>/_backup/ — helps when support copies a backup path out of context.

3. Rotation — count-based, default N=3

Policy: keep most recent N backups (both successful and failed upgrade backups counted together). Older ones pruned at the end of each upgrade run (success or failure path).

• Default N=3
• Configurable via ~/zylos/.env: COMPONENT_BACKUP_KEEP=<int> (global), or per-component via SKILL.md: backupKeep: <int> (optional, overrides global)
• Failure case is NOT specially protected — by the time we're doing rotation, rollback has already consumed the backup it needed. Keeping the most recent N is sufficient; if human investigation is still needed, the files haven't been touched yet.

Rationale for count vs. age: size-of-backup varies widely (telegram config = KB, recruit.db = MB-GB). Age-based retention can unboundedly grow disk usage for large components; count-based has a predictable ceiling.

4. Service lifecycle — component-declared preUpgradeHook / postUpgradeHook

Components that hold open file handles (SQLite WAL, lock files, long-running HTTP server) must be stopped before step2_backup copies their files; otherwise the backup is inconsistent.

Proposal: let each component declare its own lifecycle hooks in SKILL.md. The upgrade framework stays service-agnostic.

# SKILL.md
preUpgradeHook: ./scripts/pre-upgrade.sh    # optional, run before step2_backup
postUpgradeHook: ./scripts/post-upgrade.sh  # optional, run after step7_start

Contract:

• preUpgradeHook runs before step2_backup. Expected to stop any long-running services the component owns. Non-zero exit aborts the upgrade before any file is touched.
• postUpgradeHook runs after step7_start on success, or after rollback's restore on failure. Idempotent. Typical use: restart helper processes, warm caches.
• Components without hooks (pages, static SKILL-only components) skip both — zero cost.
• Framework provides a small helper library (pm2 stop <name> / pm2 start <name>) hooks can source, so 90% of components reduce to one-liners.

Why pre-upgrade hook over framework-level stop:

• Some components have no long-running service (pages, most skills) — unconditional stop adds latency and failure modes for no reason
• Some components have multi-process topologies (PM2 cluster, helper daemons) the framework can't discover generically
• Component owners know their lifecycle best; contract gives them explicit control

5. Upgrade/rollback sequence (final)

preUpgradeHook (if declared)           ← component stops its services
  ↓
step2_backup                            ← copy preserve files, write manifest.sha256
  ↓
step3_smartMerge → step4..6              ← existing flow (dependency install, migration)
  ↓                                     ← any failure falls through to rollback
step7_start                             ← new code + migration modifies dataDir
  ↓
postUpgradeHook (if declared)           ← component restarts / warms
  ↓
rotate_backups (keep last N)            ← success path cleanup

Rollback path (any failure above):
  verify manifest.sha256 → abort if mismatch (loud error, no data touched)
  restore _backup/skill/ → skillDir
  restore _backup/data/ → dataDir
  postUpgradeHook (if declared)         ← component restarts on old code
  rotate_backups (keep last N)          ← failure path cleanup (still rotates)

6. Scope for MVP

• In scope: filesystem layer — preserve declared files in skillDir + dataDir, checksum, rotation, component hooks
• Out of scope: external datastores (Postgres, Redis, S3). No component currently depends on them; when one does, propose a separate RFC for datastore-backed backup (pg_dump / RDB snapshot / object-store versioning)

7. Implementation checklist

• upgrade.js — extend step2_backup to compute + write manifest.sha256
• upgrade.js — extend rollback to verify checksums before restore; abort on mismatch
• upgrade.js — add preUpgradeHook / postUpgradeHook invocation points
• upgrade.js — add rotate_backups(keep=N) called at end of success and failure paths
• Config — support COMPONENT_BACKUP_KEEP env var; support backupKeep per-component override
• Template (zylos-component-template/template/SKILL.md) — add preUpgradeHook / postUpgradeHook / backupKeep to commented examples
• Spec (COMPONENT-SPEC.md §3) — document the three new optional fields and the hook contract
• recruit-ats — declare data://recruit.db in preserve, add preUpgradeHook that pm2 stop recruit
• telegram / lark — declare data://config.json, add preUpgradeHook for the PM2-managed services
• Integration test — upgrade-success path, upgrade-failure-rollback path, backup-corrupt-abort path, rotation-trims-to-N path

Open questions for @voyax

1. manifest.sha256 at top level of each backup dir, or co-located with each file? Top level is simpler; per-file avoids re-reading the whole manifest on partial checks. Lean top-level for MVP.
2. preUpgradeHook — synchronous (framework waits) vs async-with-timeout? Lean synchronous with a default 30s timeout, overridable per component.
3. Do we want backupKeep: 0 to mean "never rotate, keep everything"? Useful for debugging, dangerous as a default. Lean: reject 0, minimum is 1.

cc @howardzhou @voyax