Skip to content

fix(sync): record sync attempt timestamp on no-op + suppress on pull failure#1430

Open
rayers wants to merge 1 commit into
garrytan:masterfrom
rayers:fix/sync-freshness-no-op-touch
Open

fix(sync): record sync attempt timestamp on no-op + suppress on pull failure#1430
rayers wants to merge 1 commit into
garrytan:masterfrom
rayers:fix/sync-freshness-no-op-touch

Conversation

@rayers

@rayers rayers commented May 25, 2026

Copy link
Copy Markdown

Problem

doctor's sync_freshness check reads sources.last_sync_at to decide whether a source is stale. Two gaps:

(A) No-op syncs never advanced last_sync_at. When performSync returns early via the up_to_date path (git HEAD unchanged, chunker current, no detached working-tree changes), the column was never touched. So a quiet source (no upstream commits in N days) was misreported as "stale 16d" even though every cron tick polled it and got "Already up to date". The metric users intuit from "last sync" is last time we attempted a sync, not last time we imported new data.

(B) last_sync_at advanced even when the git pull failed. A network partition / revoked credential / diverged remote means we never observed remote state — but freshness still advanced, telling doctor/autopilot the source was healthy when it wasn't.

Fix

  • Advance last_sync_at = now() on the no-op up_to_date early-return (federated opts.sourceId path only; legacy config.sync.last_run path untouched).
  • Gate every last_commit write site on pullAttemptedAndFailed (threaded through writeSyncAnchor + performFullSync): last_commit/newest_content_at still advance against the stale checkout, but the freshness signal is suppressed.
  • Operator-skipped offline modes (--no-pull, detached HEAD, no origin) are NOT failures and DO advance. --dry-run stays side-effect free.

Test

test/sync-no-op-touches-last-sync-at.test.ts (PGLite, 5 cases): no-op advances freshness; no-op without sourceId is a no-write; --dry-run mutates nothing; no-op + pull-failure does NOT advance; incremental + pull-failure advances last_commit but not last_sync_at. 5/5 green on current master.


Rebased onto current master. Upstream #1794 (resumable incremental sync) reworked this path but covers neither gap — the no-op early-return at the up_to_date branch still skips the freshness write, and there's no pull-failure gate. Both halves of this fix still apply.

@rayers rayers force-pushed the fix/sync-freshness-no-op-touch branch from 01ed5e5 to f8b27a1 Compare May 26, 2026 11:10
@rayers rayers force-pushed the fix/sync-freshness-no-op-touch branch from f8b27a1 to c849560 Compare May 28, 2026 05:56
rayers added a commit to rayers/gbrain that referenced this pull request May 31, 2026
Resolve writeSyncAnchor signature conflict: PR garrytan#1430's pullFailed param and
upstream garrytan#1623's newestContentEpochMs param both added a 5th positional arg to
the same function. Merged to take both — newest_content_at (git-intrinsic HEAD
committer time) stamps regardless of pull outcome; last_sync_at (the
"observed upstream" freshness signal) stays gated on pullFailed. All 3 call
sites pass both args. tsc --noEmit clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…failure (garrytan#1430)

doctor's sync_freshness reads sources.last_sync_at to decide staleness. Two gaps:
(A) the no-op up_to_date early-return never advanced last_sync_at, so quiet
sources (no upstream commits) were misreported stale even though every cron tick
polled them; (B) last_sync_at advanced even when the git pull failed, so a
never-observed source read as fresh. Fixes both: advance on the no-op path, and
gate every last_commit write site on pullAttemptedAndFailed (threaded through
writeSyncAnchor + performFullSync). Rebased onto current master — upstream garrytan#1794
(resumable sync) reworked this path but did NOT cover either gap.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rayers rayers force-pushed the fix/sync-freshness-no-op-touch branch from c849560 to 6563dac Compare June 5, 2026 15:44
rayers added a commit to rayers/gbrain that referenced this pull request Jun 5, 2026
garrytan#1794 (garrytan#1430)

Upstream garrytan#1794 (resumable sync) covers the no-op-freshness half of garrytan#1430 but
dropped our pullFailed gate, so last_sync_at stamps even when the git pull
failed (doctor reads a never-observed source as fresh). Re-threads
pullAttemptedAndFailed through writeSyncAnchor + performFullSync. Verified:
fixes the pull-failure suppression cases in
test/sync-no-op-touches-last-sync-at.test.ts (4/5 pass; the 1 residual is a
pre-existing test-vs-garrytan#1794 mismatch on the plain no-op path, fails on the
pure-upstream base too).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rayers added a commit to rayers/gbrain that referenced this pull request Jun 5, 2026
…ock (#dcb38654) & pull-failure suppression (garrytan#1430) into local/integration

Resolves the stuck auto-ingest fork-sync: conflicts in db.ts/postgres-engine.ts/sync.ts
resolved to upstream's now-canonical fixes (f3ade6c ownership-token, ec5fed2 reconnect,
garrytan#1794 resumable-sync); two surviving local fixes upstream lacks re-grafted on top.
tsc clean; connection-resilience 25/25; garrytan#1430 suppression tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rayers added a commit to rayers/gbrain that referenced this pull request Jun 5, 2026
…_sync_at advance

The fork-sync re-graft initially carried only the pull-failure suppression half
of garrytan#1430 (mistakenly assuming upstream garrytan#1794 absorbed the no-op-advance half; it
did not — the up_to_date early-return still skips the freshness write). Adds the
missing no-op advance so doctor's sync_freshness stops misreading quiet sources
as stale. Matches the refreshed PR garrytan#1430.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rayers added a commit to rayers/gbrain that referenced this pull request Jun 8, 2026
Resolves the fork-sync conflict that the self-heal couldn't auto-resolve
(typecheck gate failed on keep-both markers in src/core/search/mode.ts).

Conflict classes:
- mode.ts / search.ts: additive on both sides — our autocut weak-top floor
  (acmts) + upstream's v0.43 relational-recall arm (rel/reld) now coexist.
  Both sides independently bumped KNOBS_HASH_VERSION 9->10; merged code carries
  BOTH contributions, so bumped to 11 to force a clean one-time cache cold-miss
  (a merged-v10 hash composition matches neither published v10).
- sync.ts: took upstream's garrytan#1794/garrytan#1970 resumable-sync rewrite wholesale, then
  re-grafted our garrytan#1430 pull-failure-freshness fix onto it (writeSyncAnchor
  pullFailed param + no-op-path last_sync_at bump). Verified upstream does NOT
  cover garrytan#1430 before re-grafting (its writeSyncAnchor is 5-arg).
- 4 test files: unioned the mode-bundle expectations + bumped KNOBS_HASH_VERSION
  assertions to 11.

Verification: typecheck clean; search-mode/knobs suites 138/138; all 31
sync test files green in isolation (garrytan#1430 behavior tests pass).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rayers added a commit to rayers/gbrain that referenced this pull request Jun 8, 2026
…ped from merge fc957c7

The merge staged upstream's sync.ts via 'checkout --theirs' but the garrytan#1430
re-graft (writeSyncAnchor pullFailed param + no-op-path last_sync_at bump +
pullAttemptedAndFailed threading) was applied to the working tree only and
never re-staged, so fc957c7 captured upstream's version WITHOUT garrytan#1430 — the
exact regression the merge resolution was meant to avoid. Tests passed because
bun reads the working tree, not the index. This commits the validated grafted
version.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant