test: currentBearer + couch manager coverage, offline CI tier, pack smoke#245
Merged
Conversation
Contributor
|
🎉 PR Validation ✅ PASSED Commit: Checks:
Ready to merge! ✨ 🔗 View workflow run |
vreshch
marked this pull request as ready for review
July 7, 2026 22:10
vreshch
added a commit
that referenced
this pull request
Jul 7, 2026
## What README overhaul plus a hand-authored architecture diagram, aligned to master after #238-#245. Ready as the npm landing page. Touches only `README.md` and `docs/**` - no `src/`, `package.json`, or workflows. ## Changes - **`docs/architecture.svg`** - hand-written SVG (no render tooling). Neutral background with dark text on light boxes, so it reads in both light and dark GitHub themes and on npm. Shows AI tools -> MCP (stdio `agentage mcp` + daemon HTTP `/mcp` on `127.0.0.1:4243`) -> CLI + local daemon (single writer) -> local vaults (git-per-vault, `@agentage/memory-core`) -> sync out to git remotes and account sync (labeled protocol-neutrally, no internal tech named). - **`README.md`** restructured for a first-time reader: What is this -> Install -> Quickstart (local-only first, then optional `agentage setup`) -> Architecture (diagram) -> Command reference -> Sync -> MCP integration -> Daemon -> Env vars -> Development. - **`docs/architecture.md`** - short walk-through of the diagram. ## Reality-check vs master Built the CLI (`npm run build`) and verified every command and flag against the live `--help` output. Corrected drift from the merged PRs: - `vault add` now defaults to an **account** vault; `--local [path]`, `--git <remote>`, `--path <dir>` are the alternatives. - `vault sync [name]` now covers git and the account channel; progress prints per vault. - Documented the `update` command and `--no-daemon` global flag. - Daemon section notes the loopback-only, token-guarded, cross-origin-rejecting API and port-in-use reporting (#243, #244). - Friendly memory errors + non-zero exits (#240); 64 KB read clamp and secret refusal. - Describes no specific `src/` file paths (a sibling PR is moving `src/sync/` paths). ## Verification - `npm run verify` green: 405 unit tests pass, type-check + lint + format:check + build clean. - Ran the offline quickstart end-to-end against the built `dist/cli.js` (vault add --local, write via stdin, list, search, read `@vault/path`) - all pass. - Rendered `docs/architecture.svg` to PNG and eyeballed legibility. Note: `format:check` only globs `{src,e2e}/**/*.ts`, so it does not lint markdown; README changes are prose-only.
vreshch
added a commit
that referenced
this pull request
Jul 7, 2026
) ## Root cause (issue #249) `src/lib/file-lock.ts` decided lock takeover from **age alone**. A holder that acquires the lock, then gets descheduled by the OS for longer than `LOCK_TTL_MS` (10s) - realistic on a 2-core GitHub runner running 20 child processes **plus** V8 coverage (added to both lanes in #245; `NODE_V8_COVERAGE` is inherited by the `execFile` children, so every child pays the tax) - still has its `<pid> <timestamp>` lock look *stale*. `acquireFileLock` (old `file-lock.ts:79-81`) then treated that live holder as crashed: ``` const held = heldAt(lockPath(target)); if (held !== null && now - held < LOCK_TTL_MS) return false; // fresh -> refuse if (!takeOverStale(target, now)) return false; // else STEAL it ``` `takeOverStale` deletes the "stale" lock and a second process enters the critical section while the original holder is still mid read-modify-write. Both `readFileSync` the array, both `push`, the later `writeFileSync` clobbers the earlier - exactly one append is lost. The holder was never dead, so it still prints `ok`: hence **all 20 children succeed yet the array is 19/20** (`expected [0..18] to deeply equal Array(20)`). This is a **product bug**, not a test bug. ## Mechanism, reproduced Standalone harness: the 20-process append race with process 0's critical section paused 11s (simulating the CPU-starvation the runner produces), against the two `file-lock.ts`: ``` ORIG pause=11000 iters=8 lostWrites=8 throwPath=0 # incl. an exact 19/20 iter FIX pause=11000 iters=10 lostWrites=0 throwPath=0 ``` The original loses an append on **every** iteration (all children still report `ok`, matching the CI signature); the fix loses none. ## The fix Takeover is now gated on **same-host pid liveness**, not age alone (`file-lock.ts` `isCrashed`): - A holder is stolen only once it is past the TTL **and** provably gone - its pid returns `ESRCH` from `process.kill(pid, 0)`, or it is our own leftover pid (we cannot be inside our own synchronous section while trying to acquire), or it is past a 60s hard backstop (guards against pid reuse after a genuine crash). - A live-but-CPU-starved holder is **waited out** by the caller's bounded backoff, never stolen. If it is genuinely wedged past `MAX_WAIT_MS`, the caller throws a loud `could not acquire lock` - a safe, visible failure, never silent corruption. Why correct: the only way two processes can be in the critical section is if a takeover fires while the first is alive. Liveness makes that impossible for any alive holder, regardless of how long the scheduler pauses it. Crash recovery is preserved (dead pid -> takeover) and the ABA takeover guard is unchanged. The lock file format stays `<pid> <timestamp>`; parsing is tolerant of legacy single-token files (parsed as pid 0 -> ages out on the TTL, so old on-disk locks still recover). Same hardening applied to `src/lib/update-lock.ts` (same lock family). It is far less exposed (10-min TTL, single-writer), but the fix is symmetric and its parser is now backward-compatible with existing bare-timestamp `update.lock` files. ## Evidence / verify - `npm run verify` green (406 tests, 48 files). - `file-lock.test.ts` looped **20x = 20/20**; **5x pinned to 2 cores** (`taskset -c 0,1`) = 5/5. - All existing tests kept, including the fail-fast-on-EACCES cases (#240) and the 20-process zero-lost-writes proof. The crashed-holder test now seeds a reaped (deterministically dead) pid instead of a hardcoded `999`, so the liveness check is exercised reliably. Added a test proving a stale-looking lock whose holder is **alive** is refused, not stolen. Closes #249
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
1 participant
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.
What
Test/CI hardening only - no
src/runtime files touched (tests + workflows + vitest config).Coverage
currentBearer(src/lib/api.ts): unexpired token returned as-is (zero network), expired token refreshes exactly once and persists the new bearer, refresh failure returns null (no throw), expired-with-no-refresh-token returns null without a network call, and signed-out returns null.src/sync/couch/manager.ts: token-acquisition failure recorded (never thrown), discovery-paused branch, re-entrant cycle no-op, cached wire + target-state reuse,rescheduletimer arming, stale-vault state pruning, manual-only (interval 0) target not armed, and the sync-on-save defer branch when signed in but the channel is not couch-ready.CI tiers
@offlinetag on the self-contained e2e files (memory-offline, vault-offline, account-vault, m1-requirements, integrity, mcp-stdio, mcp-daemon, daemon, sync, plus the two offlinestatussmokes).e2e.ymlsplit intoe2e-offline(--grep @offline, no live-stack env/secrets - a fast required signal even when dev.agentage.io is down) ande2e-live(the existing full-suite-vs-dev job).ci.yml: added annpm packsmoke job - packs the publish tarball, installs it into a temp prefix, and assertsagentage --versionequalspackage.jsonandagentage --helpexits 0 (catches a brokenbin/shebang before publish).ci.ymlmatrix widened to Node[22.x, 24.x].vitest.config.ts: per-directory coverage floors forsrc/lib/**(87/78/83/90) andsrc/sync/**(85/75/75/87), set a few points below the achieved numbers so a weak file cannot hide behind the global average. No floor added forsrc/commands/**(a sibling PR is churning those).Why
currentBeareris the background-sync bearer for the couch/manager path and was only ~85% covered; the couch manager was the least-covered real module (67.9% stmts / 51.4% funcs) with only Docker/live e2e exercising it.Coverage before -> after (statements)
src/lib/api.ts: 84.78 -> 97.82src/sync/couch/manager.ts: 67.95 -> 83.42 (funcs 51.42 -> 60.0)src/lib/**90.95 stmts / 83.57 br / 87.59 fn / 93.55 ln;src/sync/**88.89 / 80.13 / 79.49 / 90.85Verify
npm run verifygreen (350 unit tests, type-check + e2e type-check + lint + format + build).npm run test:e2e -- --grep @offlinegreen locally: 35 tests / 10 files, and confirmed the grep excludes couch-cloud, couch-sync, setup-oauth, mcp-contract.--helpexit 0.Notes
status.test.tshas 3 smokes; only the two network-independent ones (--version, signed-out no-stack-trace) got@offline. The third assertsendpoint.reachable === true, so it stays live-only.src/**runtime,src/commands/**, daemon source,package.jsondeps, orpublish.yml(siblings own them). Pack smoke is inlined in the workflow, no new package script.