Skip to content

refactor(sync): channel-based structure, split oversized modules - #247

Merged
vreshch merged 1 commit into
masterfrom
refactor/sync-structure
Jul 7, 2026
Merged

refactor(sync): channel-based structure, split oversized modules#247
vreshch merged 1 commit into
masterfrom
refactor/sync-structure

Conversation

@vreshch

@vreshch vreshch commented Jul 7, 2026

Copy link
Copy Markdown
Member

What

Pure structure/readability refactor of src/sync/. Zero behavior change - only file moves, import-path rewrites, and function extraction. Same tests pass (405), coverage floors met.

1. Channel-based sync tree

The git channel was mixed at the src/sync/ root alongside the couch/ and discover/ subfolders. Now every channel is its own folder; the root holds no logic.

Before

src/sync/
  cycle.ts  manager.ts  planner.ts  git-exec.ts  remote-url.ts  conflict.ts  (+ tests)
  couch/    discover/

After

src/sync/
  git/     cycle  manager  planner  git-exec  remote-url  conflict  (+ tests)  [git mv, history preserved]
  couch/   (channel unchanged aside from the split below)
  discover/

All imports updated across daemon-entry, daemon/server, lib/daemon-client, lib/vault-registry, commands/vault, commands/vault-sync (+ test), and the intra-couch refs to ../git/*. No shared types.ts was invented - the cross-channel types are channel-specific and stay with their channel (re-exported from couch/manager.ts to keep the public import surface stable).

2. Split oversized modules (house bar <200 lines)

src/sync/couch/manager.ts was 363 lines. Split along its natural seams into siblings, manager becomes thin composition:

file lines role
couch/manager.ts 135 composition: builds the runtime, timers, returned object
couch/manager.types.ts 104 shared type shapes (CouchRuntime, results, deps, TargetState)
couch/cycle.ts 60 one couch cycle (commit-first, flush, push+pull)
couch/wire.ts 41 getState / ensureWire / pendingCount (wire + token setup)
couch/push-on-write.ts 36 sync-on-save (push/tombstone one path)
couch/mutation-target.ts 31 verb payload -> vault + POSIX path
couch/local-commit.ts 22 default local-git commit

couch/manager.test.ts was 399 lines, split along the same seams:

file lines
couch/manager.test.ts 189 (status / runNow / reschedule)
couch/push-on-write.test.ts 113 (onWrite)
couch/mutation-target.test.ts 51
couch/manager.fixtures.ts 63 (shared test fixtures)

The extracted seams stay tested through the composed manager (integration level - they are implementation details, not a widened public API). Crash-recovery / durability tests are moved intact, semantics untouched.

3. Consistency sweep (light)

  • All files kebab-case, role suffixes where apt (.types.ts, .fixtures.ts).
  • No dead exports introduced; no stray files at src/ root.
  • src/commands/, src/lib/ left untouched per scope (commands/memory.ts at 207 is out of scope).
  • vitest.config.ts: *.fixtures.ts added to the coverage exclude (test support, not product code); src/sync/** floor glob still matches the new src/sync/git/** + src/sync/couch/** paths.

4. Docs

CLAUDE.md Layout section rewritten to describe the per-channel tree (one-line-per-path, same terseness).

Verify

npm run verify green (type-check, e2e type-check, lint, format, 405 tests, build). npm run test:coverage green - per-directory floors met (sync/git, sync/couch, sync/discover all present under the new paths). No dependency changes, no README changes.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🎉 PR Validation ✅ PASSED

Commit: 6383105c3592a24b53648b663d71fd6df6f98ef1
Branch: refactor/sync-structure

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-07T22:32:21.564Z

@vreshch
vreshch marked this pull request as ready for review July 7, 2026 22:34
@vreshch
vreshch merged commit 1d7b569 into master Jul 7, 2026
3 checks passed
@vreshch
vreshch deleted the refactor/sync-structure branch July 7, 2026 22:34
vreshch added a commit that referenced this pull request Jul 7, 2026
## Review-readiness sweep (post #238-#247)

Fresh-reviewer pass over the whole repo after the 10-PR merge burst. The
repo held up well; this PR carries the one actionable finding, plus a
clean bill of health on the rest.

### Fixed
- **`src/commands/memory.ts` was 207 lines** (over the 200 bar). Split
along its natural seam: the six `run*` verb flows + their helpers move
to `src/commands/memory-verbs.ts` (133 lines); `memory.ts` keeps the
commander wiring + error `guard` (81 lines). Test import retargeted to
`./memory-verbs.js`. No behavior change.

### Checked, nothing to fix
- **Cross-PR seams:** no stale path refs (e.g. no lingering
`src/sync/cycle.ts`), no duplicated helpers (`redactRemoteUrl` lives
once in `sync/git/remote-url.ts`, imported by all five callers).
- **File/naming bar:** all other src files <=200 lines, all kebab-case,
no default exports, no `any` outside tests.
- **Comments:** no `TODO`/`FIXME`/`XXX`, no commented-out code.
Multi-line comment blocks that remain are load-bearing
security/concurrency WHY notes (e.g. the `ext::` transport-helper RCE
guard, the cross-process `mutateVaultsConfig` invariant) - collapsing
them would lose signal, left as-is.
- **Docs accuracy:** README, `docs/architecture.md` (vs
`architecture.svg`), and CLAUDE.md layout all cross-checked against
source - 20+ claims (flags, env vars, `4243` port, `0600` auth.json, six
MCP tools, conflict-file naming) all verified, zero mismatches, zero
typos, zero em dashes.
- **CLI surface:** ran the built `dist/cli.js` help for every subcommand
in an isolated HOME - consistent tone, no typos.

`npm run verify` green; `test:coverage` green (85.5% stmts, above the
65/70/70/70 thresholds).
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