firstsun-dev_git-files-sync/progress.md
ClaudiaFang d8e3663b8f perf(delete): batch-commit remote-only file deletion
Follow-up to the push-all batching work: batch-deleting remote-only
files via the sync status panel's checkbox multi-select was still N
separate commits (SyncStatusView.performRemoteDeletion looped
gitService.deleteFile once per file).

- Add optional GitServiceInterface.deleteBatch, mirroring pushBatch.
  GitHub/Gitea implement it via the git blob->tree->commit->ref Data
  API, reusing resolveGitHubStyleBaseTree/resolveBaseTree and
  commitGitHubStyleTree (widened its tree-item sha type to
  string | null -- a null sha removes that path from the resulting
  tree, GitHub's way of expressing a delete at the tree level).
  GitLab implements it via the same Commits API endpoint pushBatch
  already uses, with action: 'delete' entries.
- SyncStatusView.performRemoteDeletion now calls deleteBatch once per
  chunk (MAX_BATCH_PUSH_SIZE, reused from the push work) when the
  provider supports it; a failed chunk marks every path in it as
  failed rather than dropping results silently. The original per-file
  loop is preserved verbatim as performRemoteDeletionSequential, the
  fallback for providers without deleteBatch.

Local deletion is unaffected -- it's a local vault operation, not a
git commit.

Evidence: npx eslint . -> 0 errors; npm run build -> clean;
npx vitest run -> 339/339 passed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-14 10:24:22 +00:00

15 KiB
Raw Blame History

Session Progress Log

Completed work is archived in archive/, one file per calendar month — this file only tracks what's still open.

Current State

Last Updated: 2026-07-14 10:30 Session ID: current Active Feature: feat-012 (perf: batch-commit remote-only file deletion) — done, lint/build/test all green. feat-011 (push-all batching) also done this session. Previous active item (issue #78 delete-remote-only-file fix) still awaiting user re-test after rebuild.

Status

What's Done

  • feat-001..009 (project setup, settings UX bundle, folder picker, symlink pull fix, tree-SHA refresh, HTML-response error clarity, what's-new modal, i18n/multi-language support) — see archive/2026-07.md

  • All consolidated onto branch claude/fix-directory-symlink-pull-260713PR #51 (open, all CI green as of last check), per user's explicit request to keep the PR count down rather than one PR per issue.

  • "Root path" folder picker now suggests folders from the remote repo tree instead of the local vault. Added src/ui/RemoteFolderSuggest.ts (derives folder paths from gitService.listFiles(branch, false)); wired into the Root path field in src/settings.ts. Vault folder field untouched (still uses local FolderSuggest).

  • Issue #78 (filed in firstsun-dev/blog but actually a git-files-sync bug — user confirmed to fix it here regardless of repo mismatch): "delete remote only file fail". Three distinct root causes found and fixed across this session (iteratively, as the user re-tested against real console errors each time):

    1. URL encoding + silent empty-sha delete: github-service.ts/gitea-service.ts getApiUrl() built the Contents API URL without encodeURIComponent on path segments (unlike gitlab-service.ts, which already encodes), so paths with spaces/non-ASCII (e.g. Chinese filenames) 404'd on the pre-delete sha lookup; that 404 silently produced an empty sha, and the resulting DELETE with sha: '' was rejected — silently, since the UI swallowed all delete errors. Fixed: encode path segments in both services; deleteFile() now throws a clear error if the pre-delete lookup's sha is empty; SyncStatusView.ts delete flow now captures {path, message} per failure and surfaces the real message in the Notice (syncStatus.notice.deleteResult.partialWithMessage i18n key, added to both en.ts/zh-tw.ts) instead of a bare "N failed".
    2. EISDIR + folder/remote-record collision: SyncStatusView.ts readFileContent()/identifyExtraFiles() treated any local path that adapter.exists() returned true for as a readable file — but a real local directory (or a symlink to one) that happens to share a path with a stale remote record (e.g. .claude/skills/polish-blog) is not readable content, and adapter.read() on it throws EISDIR. Fixed: new isLocalFile() helper (adapter.stat().type === 'file') gates identifyExtraFiles and the hidden-file recursiveScan; readFileContent's string-path branch (extracted to readStringPathContent()) also falls back to readLocalSymlinkTarget() on read failure as a last resort.
    3. Wrong path passed to deleteFile (the actual reported bug): performRemoteDeletion called gitService.deleteFile(s.path, ...) with s.path — a vault-relative path (carries the vaultFolder prefix, per identifyExtraFiles's getVaultPath() mapping) — but deleteFile's getFullPath() expects a path relative to rootPath only (vaultFolder is a purely local concept, stripped before every other gitService call site, e.g. sync-manager.ts's getNormalizedPath(file.path) pattern used everywhere else). With a non-empty vaultFolder setting, this built the wrong repo path, causing deleteFile's pre-delete getFile() lookup to 404 — surfacing as "file was not found on branch main" for a file the UI still listed as remote-only (since refresh's status computation does normalize correctly). Fixed: performRemoteDeletion now computes this.plugin.getNormalizedPath(s.path) before calling deleteFile, matching every other call site.
    • Added 4 tests for root cause #1 (tests/services/github-service.test.ts, tests/services/gitea-service.test.ts) covering the empty-sha throw and non-ASCII/space path encoding.
    • Added tests/ui/SyncStatusView.test.ts (5 cases) for root causes #2 and #3, since SyncStatusView.ts had no test file at all before this — required adding minimal ItemView/WorkspaceLeaf mocks to tests/setup.ts (constructor sets app/leaf, fakes containerEl.children[0..1]; no rendering behavior needed since tests call private methods directly via a cast rather than going through onOpen()/renderView()). Covers: vaultFolder-prefix stripping before deleteFile() (with and without a configured vaultFolder), real error messages surfacing in errors, and identifyExtraFiles correctly classifying a local-folder/remote-record collision as remote-only vs. a genuine local file as checkable.
    • Evidence: npx eslint . → 0 errors; npm run build → clean; npx vitest run → 317/317 passed.
    • All three root causes committed (896d77b, merge 563a28e, fa42fea) and pushed to claude/fix-directory-symlink-pull-260713. Test coverage for #2/#3 added after that — pending commit as of this note.
    • Not yet done: comment on/reference issue #78 (in the blog repo) noting the fix landed in git-files-sync instead — flag to user before doing so since it crosses repos.
  • feat-011: perf(push): batch-commit push-all + SHA-based diffing. User asked "現在外掛的 push功能很慢可能是什麼原因" then approved doing all three identified fixes (a/b/c) via a full plan-mode design pass. Implementation:

    1. Batched commit instead of one-commit-per-file: new optional GitServiceInterface.pushBatch?(items, branch, message). GitHubService/GiteaService implement it via the git blob→tree→commit→ref Data API (generalizing pushSymlink's existing pattern, factored into shared resolveGitHubStyleBaseTree/commitGitHubStyleTree helpers in git-service-base.ts); GitLabService implements it via the native multi-file Commits API (POST .../repository/commits with an actions array), with one follow-up listFilesDetailed call afterward to recover each file's new blob sha (that API doesn't return them). Symlinks stay on the existing per-file pushSymlink path, not folded into batches.
    2. SHA-based diffing instead of per-file getFile: sync-manager.ts's push-all flow (processPushBatch/classifyPushCandidate/classifyAgainstTreeEntry) now compares a locally-computed git blob sha (utils/git-blob-sha.ts's gitBlobSha, already used by feat-006's status refresh) against a pre-fetched remote tree's per-entry sha — no network round trip needed to detect unchanged/new/modified/conflicting files. Rename detection and symlink handling are untouched (still per-file, immediate).
    3. Dedup the remote-tree fetch: main.ts:runAllFiles now fetches the tree once (listFilesDetailed(branch, false)) and threads it into both GitignoreManager.loadGitignores(tree) (new optional param, replacing its own getRepoGitignores call when a tree is supplied) and SyncManager.pushAllFiles(files, onProgress, tree) — replacing the old discarded listFiles() call plus gitignore-manager's separate fetch with a single shared one.
    • Batches are chunked at MAX_BATCH_PUSH_SIZE = 200 files per commit call (git-service-base.ts); a failed chunk marks every file in that chunk as failed (not silently dropped), earlier successful chunks stay committed.
    • Providers without pushBatch (future Bitbucket) fall back to the original sequential per-file push path, unchanged.
    • Added pushBatch tests to github-service.test.ts/gitea-service.test.ts/gitlab-service.test.ts (happy path, empty-batch short-circuit, base64 encoding, GitLab's create-vs-update action + sha-recovery-miss case), a MAX_BATCH_PUSH_SIZE sanity test, new sync-manager-batch.test.ts cases (grouped pushBatch call, mixed binary+text batch, whole-chunk-failure, sha-match skips both getFile and pushBatch), and a gitignore-manager.test.ts case for the pre-fetched-tree path. Existing conflict/rename/symlink batch tests rewritten to mock listFilesDetailed instead of getFile for the equality/conflict check (rename detection's own getFile calls are untouched).
    • Evidence: npx eslint . → 0 errors; npm run build → clean; npx vitest run → 330/330 passed (313 pre-existing + wording/mock updates + 17 new cases).
    • Pull path (pullAllFiles) intentionally untouched — out of scope, still needs full remote content regardless of sha comparison.
    • Not yet committed as of this note — pending commit onto claude/fix-directory-symlink-pull-260713.
  • feat-012: perf(delete): batch-commit remote-only file deletion. User asked whether batch-deleting remote-only files (checkbox multi-select in the sync status panel) was also one commit; it wasn't (performRemoteDeletion looped gitService.deleteFile per file). Applied the same batching pattern as feat-011:

    1. New optional GitServiceInterface.deleteBatch?(paths, branch, commitMessage), mirroring pushBatch?. No result payload (deletes don't produce a new sha to report back).
    2. GitHubService/GiteaService.deleteBatch reuse resolveGitHubStyleBaseTree/resolveBaseTree + commitGitHubStyleTree (widened commitGitHubStyleTree's tree-item sha type to string | null — a null sha removes that path from the resulting tree, which is how GitHub's Git Data API expresses a delete at the tree level).
    3. GitLabService.deleteBatch reuses the same Commits API endpoint as pushBatch, with action: 'delete' entries (no content/encoding).
    4. src/ui/SyncStatusView.ts's performRemoteDeletion now calls deleteBatch once per chunk (MAX_BATCH_PUSH_SIZE, reused from feat-011) when the provider supports it, updating progress per-file during a fast local pass before the grouped network call — same UX pattern as push. The original per-file loop was extracted verbatim into performRemoteDeletionSequential, used as the fallback when a provider has no deleteBatch (e.g. future Bitbucket). A failed chunk marks every path in it as failed (kept in fileStatuses/selectedFiles, not silently dropped); earlier successful chunks stay deleted.
    • Local deletion (performLocalDeletion) untouched — pure local vault operation, nothing to batch.
    • Added deleteBatch tests to github-service.test.ts/gitea-service.test.ts/gitlab-service.test.ts (happy path, empty-array short-circuit) and new SyncStatusView.test.ts cases (grouped call, whole-chunk-failure, fallback when deleteBatch is absent). Existing vaultFolder-prefix-stripping and real-error-message tests still pass unchanged against the fallback path.
    • Evidence: npx eslint . → 0 errors; npm run build → clean; npx vitest run → 339/339 passed.
    • Not yet committed as of this note — pending commit onto claude/fix-directory-symlink-pull-260713.

What's In Progress

  • Nothing else actively in progress.

What's Next

  1. Get user confirmation that delete now works (or a new detailed error message) after they rebuild/reload with the latest push (issue #78).
  2. Manually verify feat-011/feat-012 in Obsidian if possible: push-all and batch-delete on a mixed vault should each produce exactly one new commit.
  3. Issue #37 (Bitbucket provider support, feat-010) — large, was deferred until #38 (i18n) landed. Now unblocked. Its GitServiceInterface implementation should simply omit pushBatch/deleteBatch and use the existing per-file fallbacks.
  4. Re-sync against gh issue list --repo firstsun-dev/git-files-sync --state open. Remaining genuinely-unstarted issues as of this session: #47 (regex ignore lists), #45 (SonarQube findings), #37 (Bitbucket), #28 (non-engineering: community visibility).
  5. PR #51 is large (8+ issues' worth of changes now). If the user wants to review/merge it before more work piles on, flag this rather than continuing to add commits indefinitely.

Blockers / Risks

  • None currently.

Decisions Made

  • All work goes into one PR (#51): user explicitly said "不要那麼多pr merge" (don't want so many PRs). Any further issue work should commit directly onto claude/fix-directory-symlink-pull-260713, not a new branch.
  • feat-009 scope (i18n): settings.ts + all Notice() messages, done in one pass rather than split into infra-first/settings-only phases.
  • Flat key-value i18n dict, not nested namespaces: matches this plugin's small scale; simpler t(key, vars) lookup, no external i18n library dependency.
  • Locale detection via window.moment.locale(): per issue #38's suggested approach; unresolvable/unsupported locales fall back to English. A bare zh locale code maps to zh-tw, the only Chinese variant shipped.
  • Diff-format markers, changelog release-note text, and proper nouns (GitHub/GitLab/Gitea) were left untranslated — out of scope for UI-chrome i18n.
  • src/changelog.ts is hand-curated, separate from the auto-generated CHANGELOG.md: semantic-release already maintains CHANGELOG.md from Conventional Commit messages, but it's commit-log-level detail; the what's-new modal needs a small, hand-written, user-facing "highlights" list instead.
  • Fixed two duplication regressions mid-session (SonarCloud gate is 3% on new code, learned the hard way on PR #49 earlier): deduped TextComponent/TextAreaComponent test mocks, and deduped the GitHub/Gitea getBlob() bodies into a shared fetchGitHubStyleBlob() base helper.

Files Modified This Session

  • Issue #78 fix: src/services/github-service.ts, src/services/gitea-service.ts, src/ui/SyncStatusView.ts, src/i18n/locales/en.ts, src/i18n/locales/zh-tw.ts, tests/services/github-service.test.ts, tests/services/gitea-service.test.ts.
  • Root path picker fix: src/ui/RemoteFolderSuggest.ts (new), src/settings.ts.
  • feat-009 (i18n, prior parallel session): src/i18n/index.ts, src/i18n/locales/en.ts, src/i18n/locales/zh-tw.ts, tests/i18n/index.test.ts (all new); src/settings.ts, src/main.ts, src/ui/SyncStatusView.ts, src/ui/SyncConflictModal.ts, src/ui/WhatsNewModal.ts, src/ui/ConfirmModal.ts, src/ui/components/ActionBar.ts, FileListItem.ts, DiffPanel.ts (modified).
  • See archive/2026-07.md entries feat-003/005/006/007/008 for earlier features' file lists.

Evidence of Completion

  • Tests pass: npx vitest run → 306/306 passed (post-merge, this session)
  • Type check clean: npm run build (tsc + Obsidian 1.11.0 compat typecheck + esbuild) → clean
  • Lint clean: npx eslint . → 0 errors
  • Manual verification in Obsidian: in progress — user is re-testing the delete fix live; not yet confirmed resolved.

Notes for Next Session

  • Working branch for all further commits: claude/fix-directory-symlink-pull-260713 (PR #51). Do not open a new branch/PR for the next issue unless the user says otherwise.
  • feature_list.json's backlog is a snapshot from this session — re-check gh issue list before trusting it.
  • If the user reports delete still failing after this push, get the exact new error message (should no longer be a bare "N failed") to find the next root cause.