Skip to content

Commit 81283af

Browse files
dbdeveloperclaude
andcommitted
refactor comments: retarget cross-refs at docs/PSEUDO-MERGE-MODE.md
The pseudo-merge design doc at docs/PSEUDO-MERGE-MODE.md is now the permanent reference. Code/test comments are rewritten so the source reads as if it had been written from that spec, with no traces of the development process: - All references to PSEUDO-MERGE-MODE-IMPLEMENTATION_PLAN.md (the work-in-progress document) replaced with cross-refs to docs/PSEUDO-MERGE-MODE.md §X.Y where the new article actually covers the topic. - All "Stage 13 / Phase 4 / Decision #X / Group N / pre-Stage-13" development-process artifacts removed. - All N-prefixed test titles (N1, N9, N13, etc., from the Phase 3 RED test plan) stripped — the test name itself describes the behaviour; the location of the spec cross-ref is the surrounding comment. - Cross-refs added only where local code can't self-explain (named protocols, non-obvious design choices, invariants the spec enforces). No reflexive "see docs/..." after every block. - Removed dead-artifact assertions: tests no longer synthesize `sibling-content.bin` files (a legacy backup file the current code never produces or reads). Test cases keep their actual behavioural pin without the legacy stand-in setup. - Same `.sync-tmp` semantic fix applied to comments in main.ts and mock-obsidian-platform.test.ts that still said `.sync-bak` for ConflictStore staging contexts. No behavioural changes; tests 526/526 GREEN; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 95d233e commit 81283af

35 files changed

Lines changed: 481 additions & 695 deletions

mock-obsidian.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ export class Vault {
147147
}
148148

149149
// Subscribe to vault events. ConflictWatcher relies on this for
150-
// its delete/modify/rename triggers — Stage 4 tests fire events
150+
// its delete/modify/rename triggers — tests fire events
151151
// synchronously via the test helper `fireEvent` below.
152152
on(event: string, cb: (...args: unknown[]) => void): EventRef {
153153
const ref = { event, cb };

src/main.ts

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,9 @@ export default class GitHubSyncPlugin extends Plugin {
246246
await queue.clearAll();
247247
}
248248
if (this.conflictStore) {
249-
// Decision #22: rename vault sibling files BEFORE dropping the
250-
// record index, so a future re-enable doesn't collide with the
251-
// user's leftover conflict-from artifacts.
249+
// Rename vault sibling files BEFORE dropping the record index,
250+
// so a future re-enable doesn't collide with the user's
251+
// leftover conflict-from artifacts.
252252
await this.conflictStore.renameVaultSiblingsToUnresolved();
253253
await this.conflictStore.clearAll();
254254
}
@@ -269,11 +269,10 @@ export default class GitHubSyncPlugin extends Plugin {
269269
lastSyncCommitSha: store.getLastSyncCommitSha(),
270270
paths: store.paths().length,
271271
});
272-
// (Stage 13 wire-up: AtomicWriteRecovery sweep moved down to AFTER
273-
// ConflictStore.load so the sweep can resolve `.sync-bak` files
274-
// owned by conflict records via record.theirsBlobSha SHA-verify,
275-
// not just snapshot-based reasoning. See block after conflictStore
276-
// construction below.)
272+
// AtomicWriteRecovery sweep runs AFTER ConflictStore.load (see
273+
// block below) so the sweep can resolve `.sync-tmp` staging
274+
// files owned by conflict records via record.theirsBlobSha
275+
// SHA-verify, not just snapshot-based reasoning.
277276
const gi = new GI(vaultRoot);
278277
const queue = new PushQueue({
279278
vault: this.app.vault,
@@ -322,7 +321,7 @@ export default class GitHubSyncPlugin extends Plugin {
322321
await conflictStore.load();
323322
this.conflictStore = conflictStore;
324323
// Crash-recovery sweep for atomic-write artifacts AND for
325-
// ConflictStore vault-level `.sync-bak` siblings. Runs BEFORE the
324+
// ConflictStore vault-level `.sync-tmp` staging siblings. Runs BEFORE the
326325
// engine starts touching the vault so any leftover staging from a
327326
// previous crash is reconciled against the snapshot + conflict
328327
// stores before findChanges or drain sees them.
@@ -337,11 +336,11 @@ export default class GitHubSyncPlugin extends Plugin {
337336
} catch (err) {
338337
await this.logger.error("Atomic-write recovery sweep failed", `${err}`);
339338
}
340-
// Stage 13 wire-up (PSEUDO-MERGE-MODE.md §"Counter formula +
341-
// vault.on listeners role"): ConflictCounter owns the count
342-
// formula + debounced recompute; ConflictWatcher just calls
343-
// counter.markDirty() on relevant vault events; the counter
344-
// notifies UI surfaces via subscribe().
339+
// ConflictCounter owns the count formula + debounced recompute;
340+
// ConflictWatcher just calls counter.markDirty() on relevant
341+
// vault events; the counter notifies UI surfaces via
342+
// subscribe(). See docs/PSEUDO-MERGE-MODE.md §5 for the layer
343+
// separation.
345344
const conflictCounter = new ConflictCounter({
346345
vault: this.app.vault,
347346
store: conflictStore,
@@ -374,8 +373,8 @@ export default class GitHubSyncPlugin extends Plugin {
374373
selfPluginId: manifest.id,
375374
// Label read live from settings so the user can change it in
376375
// the settings tab and the next sync picks up the new value —
377-
// no plugin reload needed. Stage 13: commitMessage template is
378-
// gone (Decision #36); messages are hardcoded via commit-message.ts.
376+
// no plugin reload needed. Commit messages themselves are
377+
// hardcoded in src/sync2/commit-message.ts.
379378
deviceLabel: () => this.settings.deviceLabel ?? "Obsidian",
380379
// Remote identity read live so the manager catches a mid-session
381380
// settings change (user edits the repo coords in the settings
@@ -549,12 +548,11 @@ export default class GitHubSyncPlugin extends Plugin {
549548
return false;
550549
}
551550

552-
// Refresh every visibility surface (status bar, ribbon badge,
553-
// settings tab) from the current ConflictCounter value. The
554-
// counter applies the Stage 13 formula (excludes records with
555-
// !siblingExists and records where siblingSha == baseSha) so the
556-
// UI badge reflects what the user actually still has to resolve,
557-
// not the raw record count.
551+
// Refresh every visibility surface (status bar, ribbon badge)
552+
// from the current ConflictCounter value. The counter formula
553+
// (excludes records with !siblingExists and records where
554+
// siblingSha == baseSha) reflects what the user actually still
555+
// has to resolve, not the raw record count.
558556
refreshConflictUI(): void {
559557
const count = this.conflictCounter?.getValue() ?? 0;
560558
this.conflictStatusIndicator?.refresh(count);
@@ -604,10 +602,10 @@ export default class GitHubSyncPlugin extends Plugin {
604602
if (this.statusBarItem) return;
605603
this.statusBarItem = this.addStatusBarItem();
606604
this.updateStatusBarItem();
607-
// Stage 10 — conflict-count indicator lives in its own
608-
// addStatusBarItem element so user themes can style it
609-
// independently. Click opens the first sibling in the editor
610-
// (same shortcut the pre-sync modal's "Resolve" button uses).
605+
// Conflict-count indicator lives in its own addStatusBarItem
606+
// element so user themes can style it independently. Click
607+
// opens the first sibling in the editor (same shortcut the
608+
// pre-sync modal's "Resolve" button uses).
611609
if (!this.conflictStatusIndicator) {
612610
const indicatorParent = this.addStatusBarItem();
613611
this.conflictStatusIndicator = new ConflictStatusIndicator(

src/settings/settings.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,9 @@ export interface GitHubSyncSettings {
4141

4242
enableLogging: boolean;
4343

44-
// Stage 13 (Decision #36): the `commitMessage` template field is
45-
// gone. Commit messages are hardcoded via formatX helpers in
46-
// src/sync2/commit-message.ts. The only user-tunable component of
47-
// a sync2 commit is `deviceLabel` below (the trailing suffix).
44+
// No commit-message template field. Commit messages are hardcoded
45+
// via formatX helpers in src/sync2/commit-message.ts; `deviceLabel`
46+
// below is the only user-tunable component (the trailing suffix).
4847

4948
// When sync2 is offline (last push failed) and this is true,
5049
// subsequent Sync clicks fold into the latest pending batch

src/settings/tab.ts

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,11 @@ export default class GitHubSyncSettingsTab extends PluginSettingTab {
2929
display(): void {
3030
const { containerEl } = this;
3131
containerEl.empty();
32-
// Stage 13 (Decision #21 revision): the settings-tab "Pending
33-
// conflicts" header + record list was removed. Visibility for
34-
// unresolved conflicts now lives in three places: status bar,
35-
// pre-sync modal, ribbon badge. The settings-tab variant was a
36-
// detail surface that didn't pull its weight — users with a
37-
// conflict click the status bar / ribbon to open the sibling,
38-
// not the settings tab. Detailed conflict list will live in
39-
// Diff2 (stage 2) as a dedicated UI surface.
32+
// Visibility for unresolved conflicts lives in three places:
33+
// status bar, pre-sync modal, ribbon badge. The settings tab
34+
// does NOT carry a conflict list — users with a conflict click
35+
// the status bar or ribbon to open the sibling, not the
36+
// settings tab.
4037

4138
// ── Remote repository ────────────────────────────────────────────
4239
new Setting(containerEl).setName("Remote Repository").setHeading();
@@ -287,12 +284,11 @@ export default class GitHubSyncSettingsTab extends PluginSettingTab {
287284
// ── Device identity ─────────────────────────────────────────────
288285
new Setting(containerEl).setName("Sync").setHeading();
289286

290-
// Both commit-message inputs render a live preview directly
291-
// Stage 13: commit-message template input removed (Decision #36).
292-
// Hardcoded formats live in src/sync2/commit-message.ts. The
293-
// device label is the only user-tunable component now — it
294-
// appears as the trailing " (label)" suffix on every sync2
295-
// commit. Preview shows the user what that looks like.
287+
// No commit-message template input. Hardcoded formats live in
288+
// src/sync2/commit-message.ts; the device label is the only
289+
// user-tunable component, appearing as the trailing " (label)"
290+
// suffix on every sync2 commit. The live preview below shows
291+
// the user what that looks like.
296292
const previews: Array<() => void> = [];
297293
const renderDeviceLabelPreview = (): string =>
298294
formatSyncMessage(this.plugin.settings.deviceLabel ?? "Obsidian");
@@ -397,11 +393,6 @@ export default class GitHubSyncSettingsTab extends PluginSettingTab {
397393
});
398394
});
399395

400-
// Stage 13 (Decision #36): the "Commit message" template input
401-
// is gone. Commit messages are hardcoded — see
402-
// src/sync2/commit-message.ts. Device label above is the only
403-
// user-tunable component of every sync2 commit.
404-
405396
new Setting(containerEl)
406397
.setName("Auto-canonicalize text files")
407398
.setDesc(

src/sync2/atomic-write.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,15 @@ import type SnapshotStore from "./snapshot-store";
2828
export const SYNC_TMP_SUFFIX = ".sync-tmp";
2929
export const SYNC_BAK_SUFFIX = ".sync-bak";
3030

31-
// Stage 13: pre-suffix staging path. Phase 4 Group 4 implementation.
32-
//
3331
// Computes the staging path for a target file by inserting `.sync-bak`
3432
// (or `.sync-tmp` if `which="tmp"`) BEFORE the file extension instead
3533
// of appending after it. This preserves the original extension's
3634
// visibility in Obsidian's file explorer (a `.md.sync-bak` file is
3735
// hidden under "Show all file types: false" but a `note.sync-bak.md`
3836
// is still indexed as markdown).
3937
//
40-
// Examples (from PSEUDO-MERGE-MODE.md §"Naming convention для staging
41-
// файлів — `.sync-bak` як pre-suffix"):
38+
// See docs/PSEUDO-MERGE-MODE.md §9.2 for the naming convention.
39+
// Examples:
4240
// - "Folder/note.md" → "Folder/note.sync-bak.md"
4341
// - "Plugins/foo/manifest.json" → "Plugins/foo/manifest.sync-bak.json"
4442
// - ".gitignore" → ".gitignore.sync-bak"
@@ -134,11 +132,11 @@ export async function atomicWriteFile(
134132
bytes: ArrayBuffer,
135133
afterCommit?: () => Promise<void>,
136134
): Promise<void> {
137-
// Stage 13: pre-suffix staging paths so Obsidian's file explorer
138-
// still recognizes the staging file by extension (a `.md.sync-tmp`
135+
// Pre-suffix staging paths so Obsidian's file explorer still
136+
// recognizes the staging file by extension (a `.md.sync-tmp`
139137
// file is hidden under "Show all file types: false" but a
140-
// `note.sync-tmp.md` stays visible). See PSEUDO-MERGE-MODE.md
141-
// §"Naming convention для staging файлів — `.sync-bak` як pre-suffix".
138+
// `note.sync-tmp.md` stays visible). See stagingPathFor above
139+
// and docs/PSEUDO-MERGE-MODE.md §9.2.
142140
const tmpPath = stagingPathFor(path, "tmp");
143141
const bakPath = stagingPathFor(path, "bak");
144142

@@ -210,8 +208,8 @@ interface ConflictStoreLike {
210208
// any `.sync-tmp` / `.sync-bak` leftovers and reconciles them
211209
// against the snapshot + conflict stores.
212210
//
213-
// Each suffix now has ONE consistent meaning (see PSEUDO-MERGE-MODE.md
214-
// §9 for the rationale):
211+
// Each suffix has ONE consistent meaning (see
212+
// docs/PSEUDO-MERGE-MODE.md §9 for the full rationale):
215213
//
216214
// `.sync-tmp` = NEW bytes staged for a target (existing or new).
217215
// Ambiguous between two callsites; dispatch by ownership via

src/sync2/commit-message.ts

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,12 @@
22
// Vladyslav Kozlovskyy <dbdevelop@gmail.com>, 2026.
33
// AGPL-3.0 — see LICENSE.
44

5-
// Stage 13 (Decision #36): hardcoded commit-message formats.
6-
//
7-
// Pre-Stage-13 commit messages were user-template-driven: settings
8-
// had a "Commit message" field with `{date}` / `{time}` placeholders,
9-
// rendered via `applyTemplate` and suffixed with `(deviceLabel)`
10-
// via `appendDeviceSuffix`. Stage 13 drops the whole templating
11-
// system — every commit message is now a hardcoded string with
12-
// `{deviceLabel}` as the only substitution. See PSEUDO-MERGE-MODE.md
13-
// §"Commit message formats (Stage 13 — hardcoded)".
14-
//
15-
// Why drop templates: provenance + multi-device disambiguation are
16-
// the only useful signals, both delivered by `(deviceLabel)`. Date
17-
// and time come for free from git commit metadata (authorDate /
18-
// committerDate). Power-user customization was unused and added
19-
// surface area for bugs.
5+
// Hardcoded commit-message formats. Every message is a fixed string
6+
// with `{deviceLabel}` as the only substitution; date and time are
7+
// already carried by git commit metadata (authorDate /
8+
// committerDate) and don't need to live in the message body.
9+
// Provenance + multi-device disambiguation are the only useful
10+
// signals, both delivered by the trailing `(deviceLabel)`.
2011

2112
// Sentinel used wherever sync2 needs a stand-in for an unknown device
2213
// — both at READ time (parseDeviceSuffix on a commit with no trailing
@@ -98,12 +89,11 @@ export function commitMessageForBatch(
9889
// off any commit message produced by sync2. Falls back to
9990
// UNKNOWN_DEVICE_LABEL for hand-edited or non-sync2 commits.
10091
//
101-
// This survives from the pre-Stage-13 commit-templates module
102-
// because the suffix shape didn't change — every Stage 13 formatX
103-
// function above appends `(safeLabel(...))` so the regex still
104-
// matches. Used by sync2-manager to identify the foreign device on
105-
// an existing commit (for the multi-device "who pushed this?"
106-
// observability log line).
92+
// Every formatX function above appends `(safeLabel(...))` so this
93+
// regex matches all sync2-produced messages uniformly. Used by
94+
// sync2-manager to identify the foreign device on an existing
95+
// commit (for the multi-device "who pushed this?" observability
96+
// log line).
10797
export function parseDeviceSuffix(message: string): string {
10898
const m = /\s\(([^()]+)\)\s*$/.exec(message);
10999
return m ? m[1] : UNKNOWN_DEVICE_LABEL;

src/sync2/conflict-branch.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
// Vladyslav Kozlovskyy <dbdevelop@gmail.com>, 2026.
33
// AGPL-3.0 — see LICENSE.
44

5-
// Pseudo-merge conflict-branch naming helper
6-
// (PSEUDO-MERGE-MODE.md §"Branch naming + lifecycle", stage 7a;
7-
// Decision #19).
5+
// Conflict-branch naming helper. See docs/PSEUDO-MERGE-MODE.md §4.3
6+
// for the per-device branch lifecycle and §10 Scenarios A–E for the
7+
// shape of the names in context.
88
//
99
// Pure function. Returns a bare branch name (no `refs/heads/`
1010
// prefix) with the shape:

src/sync2/conflict-classifier.ts

Lines changed: 12 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,14 @@ import { Vault } from "obsidian";
66
import { calculateGitBlobSHA } from "../utils";
77
import ConflictStore, { ConflictRecord } from "./conflict-store";
88

9-
// Pseudo-merge ConflictState classifier (PSEUDO-MERGE-MODE.md, stage 3).
10-
//
11-
// Single algorithm, per-ConflictRecord classification. Reads the live
12-
// vault + ConflictStore record cache to derive (baseExists,
13-
// siblingExists, baseSha, siblingSha) and decides one of four
14-
// outcomes (Decision below) or no-op. modify-vs-delete is NOT a
9+
// Per-ConflictRecord classifier. Implements Phase A of
10+
// `evaluateConflictState` per docs/PSEUDO-MERGE-MODE.md §5 (drain-start
11+
// pseudocode). Reads the live vault + ConflictStore record cache to
12+
// derive (baseExists, siblingExists, baseSha, siblingSha) and decides
13+
// one of three outcomes (Decision below). modify-vs-delete is NOT a
1514
// conflict kind — it auto-resolves at push-time in favor of local
1615
// modify (see conflict-detection.ts → attemptAutoMerge's "modify-
1716
// wins" outcome); the classifier never sees it.
18-
//
19-
// Per spec §"Trigger points" this is invoked from 4 places:
20-
// - drain-start sweep (Sync2Manager.drain)
21-
// - drain-end sweep (Sync2Manager.drain)
22-
// - UI op on conflict-related view
23-
// - ConflictWatcher fast-path hit (vault.on event)
2417

2518
// ── Decision (output of the pure classify() function) ─────────────────
2619

@@ -38,15 +31,6 @@ export type Decision =
3831
// baseSha content to main on path-close.
3932
| { type: "accept-theirs" };
4033

41-
// `delete-wins-cascade` was removed in Stage 13 (Decision #30 in
42-
// PSEUDO-MERGE-MODE.md). The old behavior — "user deletes base →
43-
// engine cascade-deletes every sibling for the path" — broke the
44-
// mobile delete-then-rename workflow because the cascade fired
45-
// between the user's two ops. Stage 13: row 3 returns noop; deletion
46-
// now requires the user to remove EVERY sibling for the path too
47-
// (or rename one onto the base, which removes both the sibling and
48-
// reuses the now-converged base).
49-
5034
// Pure classifier. No I/O. All inputs are already-fetched stat/hash
5135
// values.
5236
export function classify(
@@ -57,9 +41,7 @@ export function classify(
5741
siblingExists: boolean,
5842
siblingSha: string | null,
5943
): Decision {
60-
// kind is currently informational only — classify() doesn't branch
61-
// on it. Stage 13 unified the two remaining kinds (modify-vs-modify
62-
// and delete-vs-modify) under a single rule set.
44+
// `kind` is observational — classify() does not branch on it.
6345
void record.kind;
6446

6547
// !siblingExists → accept ours regardless of kind. Both
@@ -73,9 +55,12 @@ export function classify(
7355
// siblingExists from here on.
7456

7557
if (!baseExists) {
76-
// Stage 13 (Decision #30): user deleted base, sibling alive —
77-
// wait for the user to do the rest. Sibling stays; record stays.
78-
// No cascade. This unblocks delete-then-rename on mobile.
58+
// User deleted base, sibling alive — wait. Sibling and record
59+
// both stay. Engine never cascade-deletes siblings on bare
60+
// base-deletion; the user must remove every sibling for the
61+
// path too (or rename one onto the base). This is what enables
62+
// the mobile delete-then-rename workflow; see
63+
// docs/PSEUDO-MERGE-MODE.md §6.2.
7964
return { type: "noop" };
8065
}
8166

@@ -146,10 +131,6 @@ export async function evaluateConflictState(
146131
const baseExists = baseStat !== null && baseStat.type === "file";
147132
const baseSize = baseExists ? baseStat!.size : null;
148133

149-
// Stage 13 (Decision #30): the path-level Row 3 cascade is gone.
150-
// Records with `!baseExists` no longer trigger an engine-driven
151-
// sibling sweep; per-record classify() returns noop instead.
152-
153134
// Lazy base SHA: computed (or cache-hit) on first record that
154135
// needs it. After computation it's cached in `baseShaCached` for
155136
// the rest of this path's records.

0 commit comments

Comments
 (0)