Skip to content

fix(update,sync): V9 update behaviors, atomic auth, conflict window#223

Open
vreshch wants to merge 2 commits into
masterfrom
fix/update-atomicity-conflict-window
Open

fix(update,sync): V9 update behaviors, atomic auth, conflict window#223
vreshch wants to merge 2 commits into
masterfrom
fix/update-atomicity-conflict-window

Conversation

@vreshch

@vreshch vreshch commented Jul 6, 2026

Copy link
Copy Markdown
Member

Closes five verified code-vs-spec gaps around self-update, atomic auth writes, and the sync conflict crash-window. One reviewable unit; strict-TS ESM, no new deps (R6 guard green).

Fixes

1. Update source = npm registry (doc V9). fetchCliLatest now GETs https://registry.npmjs.org/@agentage/cli/latest (5s timeout, Accept: application/json, reads .version) instead of the backend /cli/latest. Same silent-null / kind:'unknown' contract on any failure; no server floor/notice so those paths simply never trigger. Callers updated (checkForUpdate(installed), status-info).

  • Tests: update-check.test.ts (registry shape, JSON Accept header, null on non-2xx/throw/no-version); status-info.test.ts mock updated to the registry route.

2. Passive check on every command + 1h TTL cache (doc V9). New src/lib/update-cache.ts: <configDir>/update-check.json ({checkedAt, latest}, 1h TTL). A commander postAction hook prints at most one dim hint (update available: X.Y.Z -> npm i -g @agentage/cli@latest) read purely from the cache, then fires a non-awaited refresh only when the cache is stale/absent. Never blocks: warm runs are cache-only (~0.15s), a cold run is bounded by a 3s background fetch, any failure is total silence. Hint suppressed for status/update (they report richer state) and under --json (no output corruption).

  • Tests: update-cache.test.ts (TTL respected = no fetch, stale = refresh, absent = refresh, throw/null = silent + no write, end-to-end cache file write, hint only when newer).
  • Smoke: node dist/cli.js --help exits ~0.15s; cold vault list warms the cache then subsequent runs are cache-only; vault list --json emits clean JSON.

3. agentage update restarts the daemon + concurrency lock (V9 + M3 "restart clears the version-mismatch warning"). After a successful install, if the daemon is running (pidfile-alive check) it is stopped and re-spawned and a line is printed; a stopped daemon is left stopped. A single-writer <configDir>/update.lock (src/lib/update-lock.ts) refuses a second run while a lock younger than 10 min is held, overrides a stale one, and is released in finally.

  • Tests: update.test.ts (restart announced only when running; lock refusal blocks install + skips release; restartDaemonIfRunning stops/starts only when running); update-lock.test.ts (acquire, refuse-fresh, override-stale, idempotent release).

4. Atomic auth.json writes. saveAuth now writes a sibling temp, chmod 0600 BEFORE the rename, then renames over (mirrors vaults.ts); dir stays 0700. No half-written or briefly-permissive token file.

  • Tests: config.test.ts (mode still 0600 after overwrite, content round-trips, no auth.json.tmp residue).

5. Conflict files durable with the merge (M4 follow-up). sync/cycle.ts reconcile previously committed the merge -X ours and only afterwards wrote + committed the .conflict.md copies, so a crash between the two left the remote side merged-away yet never surfaced. Now merge --no-commit -X ours, write the .conflict.md copies (remote blobs captured before merging, as before), then one git add -A && git commit: the merge result and the conflict copies land in a SINGLE commit. Numeric-dedup naming unchanged.

  • Tests: cycle.test.ts divergence test asserts HEAD is a merge commit (two parents) and git show --name-only of it lists note.conflict.md (window structurally closed). Added: auto-commit message matches /^sync: \d{4}-/; a file tracked before being added to ignore keeps syncing (gitignore/.git/info/exclude semantics).

Verification

  • npm run verify green (type-check + type-check:e2e + lint + format:check + 224 tests + build).
  • test:coverage green (statements 86%, branches 77%, functions 77%, lines 87%; all above the 65/70/70/70 gates).
  • Offline e2e (daemon, m1-requirements, mcp-daemon, mcp-stdio, memory-offline, sync, vault-offline): 19/19 pass against the build - including the update --check test that hits the new npm-registry source and the sync-divergence test.

Notes

  • Two sibling e2e PRs are in flight; this PR touches unit tests only (no e2e/*.test.ts changes).
  • A behavior-level e2e for offline update --check lands separately.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🎉 PR Validation ✅ PASSED

Commit: 83ea944d802980d63121819372c77292342ae0d8
Branch: fix/update-atomicity-conflict-window

Checks:

  • ✅ Release guard (no version/changelog changes)
  • ✅ Dependencies installed
  • ✅ Type check passed
  • ✅ Linting passed
  • ✅ Format check passed
  • ✅ Tests + coverage passed
  • ✅ Build successful

Ready to merge!


🔗 View workflow run
⏰ Generated at: 2026-07-06T10:04:00.287Z

@vreshch vreshch marked this pull request as ready for review July 6, 2026 09:32
@vreshch vreshch force-pushed the fix/update-atomicity-conflict-window branch from 5d28e4a to 83ea944 Compare July 6, 2026 10:03
@vreshch

vreshch commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Hardening pass (adversarial review findings, commit 2)

1 (HIGH) Offline exit delay + retry storm - fixed. Background refresh now uses node:https with an unref'd req.destroy() timer (global fetch keeps a ref'd ~10s undici connect timer alive even after its AbortSignal fires), and checkedAt persists on FAILURE too (keeping any previously known version), so an offline box retries once per hour, not per command.
Measured (registry blackholed via unroutable host, this branch's build):

scenario before after
bare fetch + 3s AbortSignal, process exit 10.5s n/a
cold cache, vault list wall time ~10.5s 3.16s (bounded, writes {checkedAt, latest:null})
fresh cache (offline) ~10.5s every command 0.14s (= node startup floor)
stale cache (offline) ~10.5s 3.15s once, then 0.15s for the next hour
mechanism: fetchLatestVersion(3000, blackholed) promise + process exit - 3.01s both
Tests: failed fetch advances checkedAt keeping prior version; first-ever failure writes latest:null; on-disk failure record throttles the next refresh; unreachable-host fetch resolves null within bound.

2 (MED) update.lock TOCTOU - fixed. writeFileSync(..., { flag: 'wx' }) is now the mutex (O_EXCL create); EEXIST -> read holder -> fresh refuses, stale unlinks and re-races exactly once. Test: 12 real concurrent processes (module transpiled on the fly, spawned children) - exactly one wins.

3 (MED) crash mid-merge wedges sync - fixed. Each cycle now aborts any interrupted operation before touching the tree: MERGE_HEAD -> merge --abort, rebase-merge/rebase-apply -> rebase --abort. Tests: simulated death between merge --no-commit and its commit, and mid-rebase - both recover (no bogus half-merge commit, conflict surfaced in the merge commit, push resumes, next cycle clean no-op).

4 (LOW) restart robustness - fixed. New stopDaemonAndWait (bounded 2s poll for the old pid to exit) runs before spawn; restartDaemonIfRunning returns restarted | failed | not-running and update prints an honest "did not restart cleanly" line on failure. Tests: stop-before-start order, failed-start reported, install-throw still releases the lock (finally), real-child stop-and-wait.

Verification: npm run verify green (234 tests, was 224); coverage 85/77/76/87 (gates 65/70/70/70); offline e2e 35/35 across the 9 master files (incl. new integrity + mcp-contract); rebased onto master (dba3de5, engine 0.1.3).

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