feat(backup): job reliability & controls + incremental backups#2599
Merged
Conversation
RunBackupWithExcludes built its own context.Background()-rooted context, so backup_stop's commandCanceller.cancelAll() had nothing to cancel for a payload-dispatched backup_run — the upload kept running after Stop. Add RunBackupContext(ctx, excludes), which derives the run's cancellable context from a caller-supplied ctx, and thread the helper's commandCanceller.track(commandID) through the backup_run case (same pattern already used by backup_restore/bmr_recover). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d backup hang) A stalled TCP connection mid-upload could wedge an entire backup job forever: the serial upload loop had no per-file deadline and the S3 client had no HTTP transport timeouts. Add a per-file upload deadline (uploadDeadline, scaled to file size with a 5-minute floor) that turns a stalled attempt into a per-file skip rather than a job abort, while still distinguishing a genuine job cancel (backup_stop) from a deadline expiry by checking the job context's Err(). Also harden the S3 client's dial/TLS/header timeouts as defense-in-depth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ver WS Adds ProgressFn to BackupManager (SetProgressFn) and threads it through CreateSnapshotContext's upload loop as createSnapshotWithProgress, which accumulates files/bytes done and invokes the callback throttled (>=3s between calls, plus one unconditional final call). RunBackupContext emits an initial totals-known call right after the file walk, before any upload completes. The breeze-backup helper wires SetProgressFn on the resolved manager in the backup_run case and forwards each call to the agent over IPC as a backup_progress envelope, mirroring the existing restore progress path so the already-wired heartbeat forwarder streams it on to the server WS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lt) behind backup_run_async capability
When the connected server advertises the backup_run_async WS capability, the
backup helper now acks a backup_run request immediately with
{"started":true} and delivers the real result later as an unsolicited
backup_result envelope, instead of blocking the agent's forward wait for the
whole run (handlers_backup_forward.go's 10-minute timeout previously bogus-
failed any longer backup while the helper kept working in the background).
Old servers see byte-identical sync behavior since the gate defaults off.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oints) BackupManager tracked lastSnapshotTime in-process and filtered the file walk to only files modified since the previous run. For server-dispatched runs the manager is fresh each command so this was a no-op, but a long-lived locally-configured manager's second snapshot would silently contain only recently-changed files while still reporting a completed, seemingly-full restore point. Remove the mechanism entirely until real incremental backups (manifest-referencing) are designed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s (locked-file fidelity) Server-dispatched backup_run payloads (managerFromBackupRunPayload) never set VSSEnabled on the BackupConfig, so Windows file-mode backups silently skipped locked files. Default it on for Windows file backups (GOOS-gated), keep it off for system_image mode (which manages its own consistency via system state collection), and let the payload's optional `vss` field override either way. VSS failure is already non-fatal, so this can't newly break Linux/macOS or Windows hosts without VSS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Exactly one retry per file, only for non-cancel failures (including a per-file deadline expiry). Job-context cancel during the first attempt or during the backoff wait aborts with errBackupStopped and is never retried. The manifest upload is unaffected — its failure stays fatal by design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The backup_result handler in heartbeat.go only logged when SendResult failed, silently orphaning the server-side job on a WS blip. Add a small on-disk outbox (backup_result_outbox.go, capped at 20 pending / 48h) that persists a terminal backup result when the send fails, and flush it on every WS reconnect via a new websocket.Client.OnConnected hook fired from the existing "connected" handshake handler. Progress messages are intentionally never outboxed — they're ephemeral. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Task 5 — adds last_progress_at (timestamptz, nullable) and total_files (integer, nullable) to backup_jobs so live upload progress and stall detection have somewhere to land. Existing transferred_size/file_count are reused for bytes-done/files-done. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…/last_progress_at) Task 6 — adds a backup_progress WS message case, extracted into services/backupProgress.ts (applyBackupProgress) so agentWs.ts stays a thin dispatcher. Validates the progress payload, enforces the same device/agent ownership check as the backup-result handler, and only updates jobs in pending|running status. total: 0 never clobbers an existing totalSize. Also adds refreshDispatchedExpectation (Redis PEXPIRE iff the key exists) to agentWorkExpectation.ts so a multi-hour backup's dispatch expectation doesn't expire before the real result arrives. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…al; advertise backup_run_async
Task 7 — two guards run before the one-shot consumeDispatchedExpectation
so a non-terminal signal can never burn the expectation the real result
needs later:
- Started-ack guard: an async-capable agent's immediate
{"started":true} command_result is treated as a progress ping
(lastProgressAt bump + expectation refresh), not a completion.
- Legacy timed-out guard: old agents' forwardToBackupHelper emits a
false "command timed out" result at exactly 10 minutes while still
uploading; today that falsely fails every backup over 10 minutes.
The Task 8 reaper now owns deciding when a silent job is actually
dead.
Also advertises backup_run_async in AGENT_WS_CAPABILITIES.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e-cap, stuck-pending)
Task 8 — reapStaleBackupJobs, registered in the stale-command-reaper
worker (every 2 min, inside withSystemDbAccessContext). A running
backup_jobs row is reaped when any of: (A) lastProgressAt stale past
15 minutes, (B) the owning device is offline (mirrors offlineDetector's
own offline-decision shape: already flipped to 'offline', or still
online/updating but past its default heartbeat threshold) and
coalesce(lastProgressAt, startedAt) stale past 10 minutes, or (C) no
progress signal was ever reported (legacy agent) and startedAt is
stale past 24h. A pending row is reaped past 60 minutes
("dispatch never completed"). Every UPDATE is guarded by
status IN ('pending','running') so a concurrent real completion always
wins. Per reaped running job on a reachable device, best-effort
queueBackupStopCommand so a live-but-silent agent stops uploading.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tsc --noEmit flagged mock.results[0] as possibly undefined; non-null assert per the existing convention in this codebase (e.g. enrollmentKeys_installer.test.ts). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds snapshotJournal (agent/internal/backup/journal.go): an append-only per-destination-identity JSONL checkpoint that records each successfully uploaded file so a later run can resume instead of restarting. Covers round-trip, last-entry-wins, corrupt-journal, and stale-journal (>maxAge) handling — all degrade to a fresh journal rather than failing a backup. Not yet wired into the upload loop; that's the follow-up commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RunBackupContext now opens a checkpoint journal keyed by destination identity (provider kind/endpoint/bucket/path + configured source paths) before creating a snapshot, and createSnapshotWithProgress uses it to: resume matched (sourcePath, size, modTime) files instead of re-uploading them, seed filesDone/bytesDone from the resumed total before the loop so progress jumps immediately, record each freshly uploaded file as it lands, and complete (delete) the journal only after the manifest upload succeeds. Stop-semantics change: every errBackupStopped exit (job cancel, per-file deadline-turned-stop, manifest-upload deadline) now skips cleanupSnapshotPrefix when a journal is active — the partial remote prefix plus the journal together are the resume state for the next run. A stale (>7d) journal instead triggers a best-effort remote prefix cleanup before the run proceeds fresh. The no-journal path (bare CreateSnapshot/ CreateSnapshotContext) keeps the old cleanup-on-stop behavior unchanged. Adds providers.JournalIdentity, an optional interface implemented by all five concrete providers (+ FallbackProvider, delegating to its primary) so journal identity can distinguish two different destinations of the same provider kind (e.g. two S3 buckets) without encoding credentials. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ity tampering Three review-flagged defects in the Task 14 checkpoint journal, all on top of f376a04: 1. VSS resume was inert: RunBackupContext rewrites walk roots to per-run shadow-copy device paths (rewritePathsForVSS), so backupFile.sourcePath is ephemeral across runs on Windows-with-VSS — keying the journal on it meant run 2 could never match anything run 1 recorded. Adds backupFile.originalPath (collector-derived via the new originalPathsForVSS, which inverts the shadow-root -> volume mapping) and SnapshotFile.OriginalPath (json:",omitempty" — byte-identical manifests when VSS is off). journal.go's Record/readJournal/Lookup now key on OriginalPath when present, else SourcePath, via journalEntryKey / journalLookupKey. 2. Identity was path-order-insensitive (sorted before hashing), but object naming is positional (path_%d by index) — reordering the configured path list between an interrupted run and its resume kept the same identity/snapshotID/prefix while silently swapping which root owns which index, so a changed file could re-upload over an object a resumed entry still referenced. backupIdentity is now deliberately order-sensitive (hashes paths in configured order): a reorder gets a fresh identity and journal instead, trading a missed resume for guaranteed-correct mapping. 3. openSnapshotJournal wrote header.Identity but never verified it against the caller's identity on open. Now checked; a mismatch gets the same treatment as staleness (discard, fresh journal, StaleSnapshotID exposed for remote cleanup). New tests: TestOriginalPathsForVSS_ReconstructsOriginalPath, TestOriginalPathsForVSS_NoOpWhenNoShadowPaths (portable — pure string manipulation, no real VSS/OS calls), TestCreateSnapshotWithProgress_VSSOriginalPathResumeMatch, TestCreateSnapshotWithProgress_NonVSSFilesCarryNoOriginalPath, TestBackupIdentity_ReorderedPathsDoNotResume, TestOpenSnapshotJournal_IdentityMismatchDoesNotResume. Updated TestBackupIdentity_DistinguishesDestinations (was asserting sort-insensitive order, now asserts the opposite by design). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # agent/cmd/breeze-backup/exec_backup_test.go
# Conflicts: # agent/internal/websocket/client.go
…r; Cancel→Stop Expose transferredSize/totalFiles/lastProgressAt through toJobResponse and render live progress (percent bar, files counter, computed transfer speed) plus a stalled badge for running backup jobs. Poll while any job runs. Legacy jobs with null progress fields fall back to an indeterminate bar. Rename the running-job action button label Cancel→Stop (handleCancel wiring unchanged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…minal guard ordering Adds four tests proving the Task 7 started-ack and legacy-timeout guards in the orphaned backup command_result path return BEFORE consumeDispatchedExpectation runs (so the one-shot expectation survives for the real terminal result), while genuine failures and completions still fall through to consume it and update the job. The extracted predicates were already unit-tested in backupProgress.test.ts; this closes the gap on the agentWs.ts integration/ordering behavior itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ncel race Address three review findings on the backup job live-progress work: - Critical: poll ticks set loading=true and the full-page spinner replaced the whole table every 5s. Gate the spinner on initial load only (loading && jobs.length === 0). - Important: the average-since-start speed fallback also fired when a prior sample existed with a <=0 byte delta, showing a positive speed on a stalled job. Use the average only when there is no prior sample; a zero/negative delta now yields no speed (cleared each refresh via a fresh speeds map). - Important: a poll GET in flight when the cancel POST landed could revert the optimistic cancel back to running. Track recently-cancelled ids (ref map, expires after ~2 poll intervals) and keep the local terminal status unless the server also reports a terminal status. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r, partial-failure visibility, progress UUID guard
Finding 1 (critical): createSnapshotWithProgress now runs a keepalive
goroutine that re-emits the current progress counters every 30s
(progressKeepaliveInterval + test seam) while a run with a non-nil
callback is in flight, so a single >15-min file upload (or the 30s retry
backoff) no longer looks dead to the server reaper and gets killed
mid-upload forever. Counters and throttle state are mutex-guarded
(progressMu); onProgress is invoked with the lock held so emissions
serialize and never go backwards; the goroutine is joined on every
return path.
Finding 2: the checkpoint journal never falls back to the
world-writable os.TempDir() (deterministic root-owned filename there is
a symlink/tamper surface). resolveJournalDir mirrors the heartbeat
outbox chain (StagingDir -> ~/.breeze -> config data dir) with test
seams; temp-dir-only environments run journal-less. openSnapshotJournal
requires a dir, creates it 0700, and Lstat-refuses (delete + fresh)
anything at the journal path that is not a regular file.
Finding 3: per-file upload failures on a partial success are no longer
silently dropped. Snapshot.UploadFailures (json:"-") carries them out;
RunBackupContext folds them into BackupJob.Warning ("N of M files
failed to upload: ..." capped at 5 details) and a new ErrorCount field;
backupCommandResultSchema + applyBackupCommandResultToJob persist
errorCount to backup_jobs.error_count (warning->errorLog already
worked; BackupJobList already renders both).
Finding 4: applyBackupProgress rejects a non-UUID commandId before any
DB access (shared UUID_REGEX in utils/uuid.ts, also adopted by
agentWs.ts), eliminating the Postgres 22P02 path and the per-ping query
for garbage ids; backup_progress drop logging is debug-level except
agent-mismatch which stays at warn.
Tests: TestSnapshotProgressKeepalive_EmitsDuringInFlightUpload (-race),
TestOpenSnapshotJournal_RefusesSymlinkJournal, resolveJournalDir suite,
TestRunBackupContext_PartialFailureSetsWarningAndErrorCount, and
API-side invalid-command-id + errorCount persistence tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… stale comment fix Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…p GC) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…vious snapshot manifest Adds manifest v2 (FormatVersion/BaseSnapshotID) and a reference decision engine (incremental.go: previousManifest, decideFile, buildPreviousIndex, isReferenceEntry, markSystemStateFiles) so createSnapshotWithProgress can skip re-uploading files unchanged since the last completed snapshot, appending a manifest entry that points at the older snapshot's object instead. Fail-open on any previous-manifest fetch/parse problem (full run, BaseSnapshotID stays empty); journal resume takes priority over a reference on the same file; system-state staging artifacts are never referenced. RunBackupContext threads the previous manifest through and surfaces BackupJob.ReferencedFiles/ReferencedBytes on the wire result. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds nullable backup_jobs.referenced_size/referenced_files columns for the agent's dedup stats (Task I1). Result schema accepts the agent's referencedBytes/referencedFiles names; persistence maps them to the DB columns (referencedBytes -> referencedSize) only when defined, copying the errorCount optional-field pattern from 2683acc so old agents that omit the fields never write NULL over 0 or vice versa. toJobResponse exposes both fields to the web layer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntal retention safety) Incremental snapshots reference objects under OLDER snapshots' prefixes (unchanged files aren't re-uploaded), so retention can no longer delete a snapshot's whole storage prefix the instant its row expires — that would destroy objects a still-retained sibling snapshot's manifest points at. - deleteSnapshotRow (backupRetention.ts) now deletes the DB row only; object deletion moves entirely to the new sweepUnreferencedBackupObjects GC phase. - Mark: live set = every backupPath + manifest key from every retained (still-existing) backup_snapshots row's manifest, per destination (backupConfigs row). Any manifest fetch/parse failure aborts the sweep for that destination only (fail-closed). - Sweep: deletes only objects not in the live set AND older than BACKUP_GC_GRACE_MS (48h, provider last-modified), including manifest-less (orphaned partial-run) prefixes once past grace. Per-destination try/catch isolation, BACKUP_GC_MAX_DELETES_PER_RUN cap (default 2000, 0=unlimited, same normalization as the stale-command reaper), delete-count logging. - backupSnapshotStorage.ts gains provider-dispatched (s3/local) listing with last-modified, object-text fetch, and keyed multi-object delete (per-key object-lock rejections counted, never thrown) — reusing the existing S3 client helper rather than adding a second access path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
sweepUnreferencedBackupObjects (added in c57d42f) was never invoked by anything — with deleteSnapshotRow correctly no longer deleting objects on row expiry, that left NO code path deleting backup objects at all (unbounded bucket growth, orphaned partials never cleaned). processCleanupExpiredSnapshots (backupWorker.ts, the cleanup-expired-snapshots repeatable job, already runs under withSystemDbAccessContext) now calls sweepUnreferencedBackupObjects() once after row-level retention has completed for every org — once, not per-org, since a destination's live set spans every retained snapshot regardless of which org's loop iteration deleted rows. The sweep is try/caught: a GC failure is logged and does not fail the retention run (row-level retention already succeeded and shouldn't be discarded/retried over an unrelated object-storage problem). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…x scoping, journal-lifetime protection, dedup-race mark Data-safety review of the mark-and-sweep GC found three Critical and two Important issues (spec amended: docs/superpowers/specs/2026-07-16-incremental-backups-design.md, "Amendments from GC review (2026-07-17)"). All fixed: CRITICAL 1 / IMPORTANT 1 — shared-bucket cross-config deletion: the sweep was scoped per backupConfigs row while listing the WHOLE bucket, so two configs pointing at one bucket could delete each other's live objects. Sweeps now run per STORAGE IDENTITY (provider + endpoint + bucket, deliberately excluding providerConfig.prefix — the agent ignores prefix when writing, a known end-to-end gap now documented on both the identity grouping and backupSnapshotRootPrefix). Retained snapshot rows are unioned across every config sharing an identity. A backup_snapshots row with a NULL config_id can't be attributed to any identity, so its mere existence blocks GC for the entire run this pass (fail-closed). CRITICAL 2 — S3 listing prefix missing trailing slash: a bare "snapshots" Prefix string-matches "snapshots-old/…" and "snapshotsummary.txt". Listing now uses "snapshots/" plus a client-side startsWith defense-in-depth filter. CRITICAL 3 — manifest-less (partial/resumable) prefixes are now protected at PREFIX granularity for BACKUP_GC_MANIFESTLESS_PREFIX_MAX_AGE_MS (7 days, mirrors the agent's journalMaxAge — cross-referencing comments added on both constants): a prefix sweeps only once its NEWEST object clears the window; a single fresh object protects the whole prefix. The 48h grace remains for loose unreferenced objects under manifest-bearing prefixes. IMPORTANT 2 — dedup-source race: the mark phase now also includes the newest manifest found in the identity's object LISTING (and its referenced keys), regardless of DB row retention, since agents pick their dedupe base from the bucket listing, not from backup_snapshots rows. MINOR — BACKUP_GC_MAX_DELETES_PER_RUN="" now treated as unset (default 2000) instead of parsing to 0=unlimited. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Completed backup jobs whose detail response carries a non-null
referencedSize now render a muted savings line in the expanded row:
"{protected} protected — {uploaded} uploaded", where protected is
totalSize and uploaded is max(0, totalSize − referencedSize) via the
existing byte formatter. Null referencedSize (legacy/full runs) renders
nothing new. Adds the `savings` key to all five locale catalogs.
Also bumps the backup.json exact-English duplicate baselines by 1 in
each translated locale to cover the pre-existing `speedValue` ("{{value}}/s")
interpolation-only literal added by the live-progress work, which had no
translatable text and was left over the baseline on this branch.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…adroom, skippedIdentities rename Second data-safety re-review found two remaining Importants, both fixed: IMPORTANT 1 — normalizeStorageIdentity: backupStorageIdentityKey previously concatenated raw config strings, so cosmetic differences (blank vs explicit default AWS endpoint, endpoint trailing slash + host case, scheme-less vs schemed, local path spelling) could split one physical bucket into two identities and resurrect cross-config deletion (CRITICAL 1). Replaced with normalizeStorageIdentity: canonicalizes a blank S3 endpoint and the explicit default AWS endpoint to the same value, parses host+port only (ignoring scheme/path/trailing-slash, forcing https when scheme-less), lowercases the host, and resolves local paths via path.resolve (collapses trailing slash, double slashes, "." segments). Added belt-and-braces detectSuspiciousStorageIdentityCollisions: a cruder bucket+hostname-only comparison flags any pair of DIFFERENT computed identities that still look like the same physical bucket (e.g. an unhandled implicit-vs-explicit-port variant) and fail-closed-skips all of them for the run, so an unanticipated gap in normalization can never run two overlapping sweeps. IMPORTANT 2 — manifest-less prefix threshold corrected from journalMaxAge (7 days) to journalMaxAge + 48h resume headroom (9 days): a resume opened just inside the journal window (e.g. day 6.9) legitimately keeps running past day 7, so equality left a boundary race where GC could sweep a still-live resume's manifest-less prefix. Cross-referencing comments updated on both BACKUP_GC_MANIFESTLESS_PREFIX_MAX_AGE_MS (backupRetention.ts) and journalMaxAge (agent/internal/backup/journal.go) — the Go comment now states the GC threshold must stay strictly larger, not merely equal. Minors: BackupGcResult.skippedDestinations renamed to skippedIdentities throughout (backupRetention.ts, backupWorker.ts's gcSkippedIdentities, and tests) to match GC's actual unit of work post-CRITICAL-1. The null-config_id fail-closed log is now explicit about required remediation (attribute or delete the orphaned row) since nothing auto-heals it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… Redis result path Two bugs found by live E2E on the incremental-backup branch: - backup_stop on a device with no agent.yaml backup config (every policy-managed device) hit the helper's nil-mgr "backup not configured" fallback before reaching the command canceller, so cancelled jobs kept uploading to completion and wrote their manifests. Route backup_stop through commandCanceller.cancelAll() in the nil-mgr switch. - The WS backup-result handler's enqueueBackupResults payload omitted referencedFiles/referencedBytes/errorCount (and the strict queue schema rejected them), so incremental upload savings and partial-failure counts persisted as NULL whenever Redis was available — only the no-Redis inline fallback kept them. Add the fields to the enqueue payload, the ProcessResultsResult interface, and backupProcessResultSchema. Full E2E evidence in docs/testing/FEATURE_TEST_LOG.md (2026-07-17 entry). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses the findings from a multi-agent pre-PR review of the backup reliability + incremental-backups work. Critical + Important fixes span the agent (Go), API (TS), and web layers, plus test-gap coverage and a cross-layer contract test. Critical - Agent-side retention no longer prefix-deletes snapshots that incremental manifests still reference. Pruning is disabled whenever incremental dedupe is active (server GC is the sole retention authority); reference-aware agent-side pruning is deliberately deferred. Regression test proves no stranded objects. Delivery loss (agent) - Terminal backup results are no longer dropped when wsClient is nil or when writePump fails: results route through a dedicated resultChan with an OnResultWriteFailed hook that re-enqueues to the outbox for reconnect flush. - Outbox filename hardened against path traversal in server-supplied CommandID. - Collection-phase (scan) errors now feed ErrorCount/Warning so a partial backup no longer reports as a green, zero-error job. GC correctness (API) - Mark phase protects every listed manifest (not just newest-by-S3-clock), closing the dedup-base race against the agent's internal-timestamp ordering. - Retained rows filtered to file-type snapshots so a single system_image/ hyper-v/mssql snapshot sharing a bucket can't permanently wedge an identity; adds a distinct blockedIdentities signal. captureException wired for GC failures. Reaper / progress (agent + API) - Whole-run progress keepalive so long VSS/prep phases don't trip the 15-min stall reaper. - A late completed result flips a reaper-failed job back to completed and creates the snapshot row (never resurrects a user cancel). - Reaper pending-branch respects lastProgressAt; zombie rows with NULL timestamps are reapable via COALESCE(...createdAt). Web - Cancel handler adopts runAction and surfaces the HTTP-200 "stop signal not delivered — may still be running" warning instead of showing a clean success. Hardening / suggestions - IN_FLIGHT_BACKUP_JOB_STATUSES deduplicated into the schema. - started_at/completed_at aligned to timestamptz via forward migration. - journal Complete() poisons the journal if removal fails (no resume into a completed snapshot). - Corrected stale comment cross-references; fixed binding spec to match the safer shipped fail-closed behavior; stripped review-artifact comment labels. Tests - Cross-layer contract test pinning the Go<->TS journalMaxAge / snapshots-root / result-JSON contracts. - Local + S3 GC I/O, refreshDispatchedExpectation, de-tautologized VSS default, reaper integration + boundary pins, corrupt-manifest parse, applyBackupProgress terminal-race, outbox flush ordering/wiring, web terminal-race. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying breeze with
|
| Latest commit: |
309960f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f73a2f72.breeze-9te.pages.dev |
| Branch Preview URL: | https://toddhebebrand-backup-work.breeze-9te.pages.dev |
…work # Conflicts: # apps/web/src/lib/i18n/translationCoverage.test.ts # docs/testing/FEATURE_TEST_LOG.md
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- security (CodeQL go/clear-text-logging, high): the backup journal identity embeds provider credentials (S3 secret key); the identity-mismatch warning logged it in clear text. Log a sha256 fingerprint instead. - golangci-lint (new-issues-only): check 17 previously-unchecked error returns (errcheck) across journal/websocket/sessionbroker/backup test + prod paths, gofmt the resultChan struct field, and apply De Morgan to the incrementalDedupeActive guard (staticcheck QF1001). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Summary
Backup job reliability + controls and incremental backups, across the agent (Go), API (Hono/Drizzle), and web layers, hardened by a multi-agent pre-PR review pass (final commit).
Reliability & controls
backup_progressintobackup_jobs(bytes/files/last_progress_at); a stalled/orphaned-job reaper fails jobs that stall, go offline, or exceed an absolute cap; started-ack and legacy-timeout results are treated as non-terminal.Incremental backups
Migrations
2026-07-16-backup-job-progress.sql,2026-07-16-z-backup-job-referenced.sql(progress + referenced columns).2026-07-17-align-backup-jobs-timestamptz.sql(alignsstarted_at/completed_attotimestamptz; idempotent, guarded on column type — no rewrite on UTC hosts).Pre-PR review fixes (final commit)
A multi-agent review (correctness, silent-failure, tests, comments, types) surfaced issues concentrated in the agent-local path and delivery windows. All addressed:
Critical
Important
wsClientis nil orwritePumpfails (dedicatedresultChan+OnResultWriteFailed→ outbox).system_image/hyper-v/mssqlsnapshot can't permanently wedge an identity (addsblockedIdentitiessignal).ErrorCount/Warning(no green job over skipped files).completedresult flips a reaper-failed job back to completed and records its snapshot (never resurrects a user cancel).runActionand surfaces the "may still be running" warning.Hardening / tests
IN_FLIGHT_BACKUP_JOB_STATUSESdeduplicated into the schema;captureExceptionwired for GC; journalComplete()poisons on remove-failure; stale comment cross-refs corrected; binding spec fixed to match the safer shipped behavior.journalMaxAge/ snapshots-root / result-JSON contracts; plus local+S3 GC I/O, de-tautologized VSS test, reaper integration + boundary pins, corrupt-manifest,applyBackupProgressrace, outbox flush ordering/wiring, web terminal-race.Verification
go build ./...,go vet,go test -race— all backup/heartbeat/websocket/cmd packages green.tsc --noEmitclean; 218 unit tests + reaper integration test (real Postgres) + migration-ordering regression green.Known deferred / residual
WriteMessagereturns nil but the server never processes before the socket drops (needs an app-level per-result ACK).backup_jobs.createdAt/updatedAtremain plaintimestamp(table-wide convention); the reaper's pending-branchcreatedAtcompare shares the same latent non-UTC skew — flagged for follow-up.🤖 Generated with Claude Code