firstsun-dev_git-files-sync/progress.md

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:10 Session ID: current Active Feature: feat-011 (perf: batch-commit push-all + SHA-based diffing) — done, lint/build/test all green. 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-260713 → PR #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.

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 in Obsidian if possible: push-all on a mixed vault should produce one new commit containing only changed/new files.
3. Issue #37 (Bitbucket provider support, feat-010) — large, was deferred until #38 (i18n) landed. Now unblocked. Its GitServiceInterface implementation should simply omit pushBatch and use the existing per-file fallback.
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.