From 45924cbcd7ce102fa2e5fa5459b0f0a229cd7479 Mon Sep 17 00:00:00 2001 From: mikael-lovqvists-claude-agent Date: Sat, 7 Mar 2026 01:41:25 +0000 Subject: [PATCH] Add rsync exit code awareness + plan operation abstraction - spawn.js: rsync() wrapper handles exit codes 0/24 as OK, 23 as fatal - spawn.js: capture() accepts allowedExitCodes option - run.js: all rsync calls go through rsync() wrapper - PLAN.md: document planned operation abstraction refactor --- PLAN.md | 34 ++++++++++++++++++++++++++++++++++ lib/commands/run.js | 24 +++++++++--------------- lib/spawn.js | 42 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 83 insertions(+), 17 deletions(-) diff --git a/PLAN.md b/PLAN.md index faf8edd..092ee40 100644 --- a/PLAN.md +++ b/PLAN.md @@ -127,6 +127,40 @@ CWD or implicit defaults for directories — explicit is safer. All external tools (rsync, zstd, xdelta3) are spawned with explicit argument arrays. No shell string interpolation ever. Use Node's `child_process.spawn` or similar. +### Planned: Operation Abstractions + +Currently dry-run logic is scattered inline throughout the run command. The intent is to refactor +toward self-describing operation objects — each operation knows both how to describe itself (for +dry-run) and how to execute itself. This makes the run command a clean sequence of operations, +makes per-tool behavior easy to adjust (e.g. rsync exit code handling), and makes dry-run output +a natural consequence of the abstraction rather than duplicated conditional logic. + +Sketch: +```js +// Each tool gets its own operation type +const op = rsyncOp({ args: [...], allowedExitCodes: [0, 24] }); +op.describe(); // prints what it would do +await op.run(); // executes + +// Run command becomes: +const ops = buildOps(config); +if (dry) ops.forEach(op => op.describe()); +else for (const op of ops) await op.run(); +``` + +Per-tool exit code handling (e.g. rsync's partial transfer codes) lives inside the operation, +not scattered across callers. + +### Current: rsync Exit Code Handling + +rsync meaningful exit codes: +- `0` — success +- `23` — partial transfer due to error (fatal) +- `24` — partial transfer due to vanished source files (acceptable in some cases) + +Currently basic: any non-zero exit code throws. Finer-grained handling planned as part of the +operation abstraction refactor. + ## Occasional Snapshots Delta chains are efficient but fragile over long chains. Periodic full snapshots (every N deltas, diff --git a/lib/commands/run.js b/lib/commands/run.js index d225036..440ee5c 100644 --- a/lib/commands/run.js +++ b/lib/commands/run.js @@ -3,7 +3,7 @@ */ import { mkdir, writeFile } from 'fs/promises'; import { join } from 'path'; -import { run as spawn, capture } from '../spawn.js'; +import { run as spawn, rsync } from '../spawn.js'; import { parseItemize } from '../itemize.js'; import { getBackend } from '../backends/index.js'; import { readState, writeState, PHASES } from '../state.js'; @@ -33,28 +33,22 @@ export async function runCommand(config) { // ── Phase 2: rsync PREV → PEND (local seed, with delete) ──── await setPhase(deltas, state, PHASES.RSYNC_LOCAL, dry); console.log('\n── rsync PREV → PEND (local seed) ──'); - await spawn('rsync', ['-aP', '--delete', trailingSlash(prev), trailingSlash(pend)], { dryRun: dry }); + await rsync(['-aP', '--delete', trailingSlash(prev), trailingSlash(pend)], { dryRun: dry }); // ── Phase 3: rsync SOURCE → PEND, capture change list ─────── await setPhase(deltas, state, PHASES.RSYNC_REMOTE, dry); console.log('\n── rsync SOURCE → PEND ──'); - let changes = []; + const output = await rsync( + ['-aP', '--itemize-changes', '--delete', trailingSlash(source), trailingSlash(pend)], + { dryRun: dry, capture: true }, + ); + const changes = dry ? [] : parseItemize(output); if (!dry) { - const rsyncArgs = [ - '-aP', - '--itemize-changes', - '--delete', - trailingSlash(source), - trailingSlash(pend), - ]; - const output = await capture('rsync', rsyncArgs); - changes = parseItemize(output); console.log(` ${changes.length} file(s) changed`); for (const c of changes) console.log(` [${c.status}] ${c.path}`); } else { - console.log(`$ rsync -aP --itemize-changes --delete ${trailingSlash(source)} ${trailingSlash(pend)}`); - console.log('[dry-run] (change list will be determined at runtime)'); + console.log(' [dry-run] change list determined at runtime'); } // ── Phase 4: Generate per-file deltas into DELTAS/tmp/N/ ──── @@ -132,7 +126,7 @@ export async function runCommand(config) { // ── Phase 6: Promote PEND → PREV ──────────────────────────── await setPhase(deltas, state, PHASES.PROMOTING, dry); console.log('\n── Promote PEND → PREV ──'); - await spawn('rsync', ['-aP', '--delete', trailingSlash(pend), trailingSlash(prev)], { dryRun: dry }); + await rsync(['-aP', '--delete', trailingSlash(pend), trailingSlash(prev)], { dryRun: dry }); // ── Done ───────────────────────────────────────────────────── state.last_complete = seq; diff --git a/lib/spawn.js b/lib/spawn.js index 7fa812b..5934ae3 100644 --- a/lib/spawn.js +++ b/lib/spawn.js @@ -29,9 +29,10 @@ export async function run(cmd, args, { dryRun = false } = {}) { * stderr is inherited (shown to user). Never used in dry-run context. * @param {string} cmd * @param {string[]} args + * @param {{ allowedExitCodes?: number[] }} opts * @returns {Promise} */ -export async function capture(cmd, args) { +export async function capture(cmd, args, { allowedExitCodes = [0] } = {}) { console.log(`$ ${[cmd, ...args].join(' ')}`); return new Promise((resolve, reject) => { @@ -40,8 +41,45 @@ export async function capture(cmd, args) { child.stdout.on('data', chunk => chunks.push(chunk)); child.on('error', reject); child.on('close', code => { - if (code === 0) resolve(Buffer.concat(chunks).toString('utf8')); + if (allowedExitCodes.includes(code)) resolve(Buffer.concat(chunks).toString('utf8')); else reject(new Error(`${cmd} exited with code ${code}`)); }); }); } + +// rsync exit codes that are not errors +const RSYNC_OK_CODES = [ + 0, // success + 24, // partial transfer: source files vanished mid-run (acceptable) +]; + +const RSYNC_ERROR_CODES = { + 23: 'partial transfer due to error', +}; + +/** + * Run rsync with exit code awareness. + * @param {string[]} args + * @param {{ dryRun?: boolean, capture?: boolean }} opts + * @returns {Promise} + */ +export async function rsync(args, { dryRun = false, capture: doCapture = false } = {}) { + console.log(`$ rsync ${args.join(' ')}`); + if (dryRun) return doCapture ? '' : undefined; + + return new Promise((resolve, reject) => { + const stdio = doCapture ? ['inherit', 'pipe', 'inherit'] : 'inherit'; + const child = spawn('rsync', args, { stdio }); + const chunks = []; + if (doCapture) child.stdout.on('data', chunk => chunks.push(chunk)); + child.on('error', reject); + child.on('close', code => { + if (RSYNC_OK_CODES.includes(code)) { + resolve(doCapture ? Buffer.concat(chunks).toString('utf8') : undefined); + } else { + const reason = RSYNC_ERROR_CODES[code] ?? `unknown error`; + reject(new Error(`rsync exited with code ${code}: ${reason}`)); + } + }); + }); +}