mirror of
https://github.com/firstsun-dev/git-files-sync.git
synced 2026-07-22 06:54:27 +00:00
chore: update harness state for GraphQL push + post-push status fix
Records feat-014 (GitHub GraphQL batch push/delete, commit114a575) and feat-015 (avoid stale remote-tree read after push, commit7676325) as done. Also archived feat-011/012/013 and other completed narratives out of progress.md (had grown past its own 80-line cleanup threshold) into archive/2026-07.md, and trimmed feature_list.json evidence strings under 300 chars — harness self-audit was 96/100, now 100/100.
This commit is contained in:
parent
7676325088
commit
4fb27854fa
4 changed files with 71 additions and 91 deletions
|
|
@ -13,3 +13,8 @@ over — don't let a single archive file grow without bound either.
|
|||
- feat-007: Clear error when requestUrl() itself rejects with an HTML body (#31) — some Obsidian versions eagerly JSON-parse inside requestUrl(), so a login/proxy HTML page rejected the whole call with a raw SyntaxError bypassing the existing parseJson() HTML-detection wrapper (commit a867217)
|
||||
- feat-008: "What's new" modal after a version bump (#39) — hand-curated src/changelog.ts (separate from auto-generated CHANGELOG.md) with a `notable` flag per entry, numeric compareVersions(), WhatsNewModal shown once per upgrade (commit 4eebebc)
|
||||
- All six of the above (feat-002/003/005/006/007/008) consolidated onto branch claude/fix-directory-symlink-pull-260713 → PR #51 (open, not yet merged) to keep the PR count down per user request; PRs #49/#50/#52/#53 closed/auto-resolved as superseded
|
||||
- feat-011: perf(push): batch-commit push-all + SHA-based diffing — new GitServiceInterface.pushBatch, sync-manager classifies files via local git blob SHA vs. one pre-fetched tree instead of per-file getFile (commit c28e0ec)
|
||||
- feat-012: perf(delete): batch-commit remote-only file deletion — new GitServiceInterface.deleteBatch, mirrors pushBatch (commit d8e3663)
|
||||
- Issue #78: remote-repo root path picker now suggests from the remote tree, delete-remote-only-file silent failures fixed (3 root causes: unencoded path 404s, EISDIR on folder/remote-record collisions, wrong vaultFolder-prefixed path passed to deleteFile), SyncStatusView test coverage added (commits 896d77b, 563a28e merge, fa42fea, f1420f0)
|
||||
- feat(ui): persistent connection status badge in the global status bar, plus a theme-contrast follow-up fix (commits 83499c9, 12cce64)
|
||||
- feat-013: perf(push): parallelize blob creation within a batch commit — BaseGitService.mapWithConcurrency + BLOB_CREATE_CONCURRENCY=8 for GitHub/Gitea's pushBatch (GitLab unaffected, already single-request) (commit c7ae0f6)
|
||||
|
|
|
|||
|
|
@ -72,7 +72,7 @@
|
|||
"description": "Extract hardcoded UI strings (settings tab, ribbon/command labels, Notice messages, sync status view/modals) into an i18n key system with locale detection and English fallback",
|
||||
"dependencies": [],
|
||||
"status": "done",
|
||||
"evidence": "Commit 144eb28 on branch claude/i18n-support-260713, merged into claude/fix-directory-symlink-pull-260713 - added src/i18n/{index,locales/en,locales/zh-tw}.ts (159 keys, en + zh-tw) and tests/i18n/index.test.ts; ~130 strings replaced across settings.ts, main.ts, and 7 ui/ files; lint/build clean, 308 tests pass; consolidated onto PR #51"
|
||||
"evidence": "Commit 144eb28, merged into claude/fix-directory-symlink-pull-260713 - 159 i18n keys (en+zh-tw), ~130 strings replaced; lint/build clean, 308 tests pass; consolidated onto PR #51"
|
||||
},
|
||||
{
|
||||
"id": "feat-010",
|
||||
|
|
@ -105,6 +105,22 @@
|
|||
"dependencies": ["feat-011"],
|
||||
"status": "done",
|
||||
"evidence": "npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 340/340 passed; consolidated onto PR #51"
|
||||
},
|
||||
{
|
||||
"id": "feat-014",
|
||||
"name": "perf(push): GitHub batch push/delete via GraphQL createCommitOnBranch",
|
||||
"description": "User reported batch push was still slow on GitHub after feat-013's blob-concurrency fix. Root cause: GitHubService.pushBatch/deleteBatch still needed N/8 rounds of sequential REST blob-creation calls before the tree/commit/ref sequence. Replaced GitHub's pushBatch/deleteBatch with a single GraphQL createCommitOnBranch mutation carrying file content directly (additions/deletions), cutting an N-file batch to 2-3 HTTP calls total (ref lookup + mutation, plus one follow-up tree fetch for pushBatch's per-file shas, which the mutation doesn't return). Extracted BaseGitService.getLatestCommitSha out of resolveGitHubStyleBaseTree (still used by pushSymlink and Gitea's REST-only batch path, which has no GraphQL equivalent). Added explicit handling for GraphQL's 200-status-with-errors-array failure mode, which REST's status-code check can't catch.",
|
||||
"dependencies": ["feat-013"],
|
||||
"status": "done",
|
||||
"evidence": "Commit 114a575. npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 341/341 passed; live smoke test against a real GitHub repo confirmed pushBatch/deleteBatch each produce one commit."
|
||||
},
|
||||
{
|
||||
"id": "feat-015",
|
||||
"name": "fix(push): avoid stale remote-tree read right after a batch push",
|
||||
"description": "User reported: batch-pushing two empty files, then refreshing immediately, showed them as 'modified' even though the diff was identical; refreshing again a bit later showed them synced. Root cause (confirmed via live GitHub API test, not a content bug - the pushed blob was byte-for-byte correct): SyncStatusView re-fetched the remote tree via refreshAllStatuses() right after push, and GitHub's tree-by-branch-name read can lag a just-completed write by a moment. Fix: SyncManager.pushAllFiles now also returns syncedPaths (path + new sha) from data already known at each write site (batch chunk result, sequential fallback, immediate symlink/rename push); SyncStatusView marks those paths 'synced' directly instead of re-fetching the tree, avoiding the race entirely. Pull unaffected.",
|
||||
"dependencies": ["feat-014"],
|
||||
"status": "done",
|
||||
"evidence": "Commit 7676325. npx eslint . -> 0 errors; npm run build -> clean; npx vitest run -> 344/344 passed."
|
||||
}
|
||||
],
|
||||
"_evidenceStyle": "Keep evidence to one line: commit hash + short pointer (e.g. 'Commit abc1234 - added X, tests pass'). Debugging narrative and design discussion belong in the commit message, not this file."
|
||||
|
|
|
|||
92
progress.md
92
progress.md
|
|
@ -12,68 +12,32 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont
|
|||
|
||||
## Current State
|
||||
|
||||
**Last Updated:** 2026-07-14 10:45
|
||||
**Last Updated:** 2026-07-14 12:05
|
||||
**Session ID:** current
|
||||
**Active Feature:** feat-013 (perf: parallelize blob creation in batch push) — done, lint/build/test all green. feat-011/feat-012 (push-all + delete batching) also done this session. Previous active item (issue #78 delete-remote-only-file fix) still awaiting user re-test after rebuild.
|
||||
**Active Feature:** None — feat-014 and feat-015 both committed. Ready for the next issue.
|
||||
|
||||
## Status
|
||||
|
||||
### What's Done
|
||||
|
||||
- [x] 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.
|
||||
- [x] "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`).
|
||||
- [x] 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.
|
||||
|
||||
- [x] 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`.
|
||||
|
||||
- [x] 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`.
|
||||
|
||||
- [x] feat-013: perf(push): parallelize blob creation within a batch commit. User reported "batch push 還是很慢" (batch push is still slow) after feat-011/feat-012 landed; confirmed they were on GitHub with the latest build. Root cause found by re-reading `GitHubService.pushBatch`/`GiteaService.pushBatch`: the blob-creation step was still a sequential `for` loop — one `POST .../git/blobs` awaited fully before the next started — so wall-clock time for the batch was still O(N) network round trips of pure latency, even though only one commit was produced at the end. This was a known, called-out tradeoff from feat-011's plan ("sequential is fine... could be Promise.all'd for extra speed, but... note as a possible future optimization"); the user's report made it worth doing now.
|
||||
- Added `BaseGitService.mapWithConcurrency<T,R>(items, concurrency, fn)` (`git-service-base.ts`): order-preserving, bounded-concurrency mapper (worker-pool pattern over a shared cursor, `Promise.all` over `Math.min(concurrency, items.length)` workers).
|
||||
- Added `BLOB_CREATE_CONCURRENCY = 8` constant alongside `MAX_BATCH_PUSH_SIZE`.
|
||||
- `GitHubService.pushBatch`/`GiteaService.pushBatch` now build blobs via `mapWithConcurrency` instead of a `for` loop; the tree/commit/ref sequence after it is unchanged (genuinely sequential — each step depends on the previous one's result).
|
||||
- GitLab unaffected — its `pushBatch` already sends the whole batch in one Commits API request, no per-file blob step to parallelize.
|
||||
- Added a regression-guard test (`github-service.test.ts`): deferred blob-response promises that only resolve after every blob POST has been dispatched — a reintroduced sequential loop would deadlock on this test (call 2 never fires until call 1's promise resolves, which it doesn't), so it fails loudly if someone reverts to sequential.
|
||||
- Evidence: `npx eslint .` → 0 errors; `npm run build` → clean; `npx vitest run` → 340/340 passed.
|
||||
- Not yet committed as of this note — pending commit onto `claude/fix-directory-symlink-pull-260713`.
|
||||
- [x] feat-001..013 — see [archive/2026-07.md](./archive/2026-07.md)
|
||||
- [x] feat-014: GitHub's `pushBatch`/`deleteBatch` now use GraphQL `createCommitOnBranch` instead of the REST blob-per-file loop (commit `114a575`).
|
||||
- [x] feat-015: fixed a false "modified" status right after batch push, caused by GitHub's tree-by-branch-name read lagging a just-completed write. `pushAllFiles` now returns `syncedPaths`; `SyncStatusView` marks those files synced directly instead of re-fetching the remote tree (commit `7676325`).
|
||||
- Both committed onto `claude/fix-directory-symlink-pull-260713`, **not yet pushed to remote**.
|
||||
- Evidence: `npx eslint .` → 0 errors; `npm run build` → clean; `npx vitest run` → 344/344 passed.
|
||||
|
||||
### What's In Progress
|
||||
|
||||
- Nothing else actively in progress.
|
||||
- Nothing 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/feat-013 in Obsidian if possible: push-all and batch-delete on a mixed vault should each produce exactly one new commit, and push-all should now feel noticeably faster on GitHub/Gitea (blobs created concurrently, up to 8 in flight).
|
||||
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.
|
||||
1. Push `claude/fix-directory-symlink-pull-260713` to update PR #51 — confirm with the user first.
|
||||
2. Manually verify feat-014/feat-015 inside the actual Obsidian plugin UI (this session's verification used a live GitHub API smoke test driving `GitHubService` directly, not the full plugin).
|
||||
3. Filed issue #57 (repo `git-files-sync`, Project #6, P2, 4h estimate): build a repeatable live-credential smoke test process across GitHub/GitLab/Gitea, not just GitHub — https://github.com/firstsun-dev/git-files-sync/issues/57
|
||||
4. Re-sync against `gh issue list --repo firstsun-dev/git-files-sync --state open` — `feature_list.json`'s backlog is a snapshot from an earlier session.
|
||||
5. Issue #37 (Bitbucket provider support, feat-010): unblocked, still not started.
|
||||
6. PR #51 is large (10+ issues' worth of changes now). Flag to the user before piling on more commits.
|
||||
|
||||
## Blockers / Risks
|
||||
|
||||
|
|
@ -81,30 +45,24 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont
|
|||
|
||||
## 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.
|
||||
- **All work goes into one PR (#51)**: user explicitly said "不要那麼多pr merge" (don't want so many PRs). Commit new work directly onto `claude/fix-directory-symlink-pull-260713`.
|
||||
- **Optimistic local status update over a post-push re-fetch (feat-015)**: rather than delaying the refresh or retrying, mark just-pushed files synced from data already in hand. Correct by construction (it's the exact content just written) and sidesteps GitHub's eventual-consistency window entirely instead of just narrowing it.
|
||||
- **Credential handling for live smoke tests**: write PATs to a local scratchpad file the agent reads directly — never as a `!`-prefixed command (still lands in the transcript) or a Bash argument (blocked by the permission classifier). Filed as issue #57 to build a proper reusable process across all three providers.
|
||||
|
||||
## 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.
|
||||
- `src/services/github-service.ts`, `src/services/git-service-base.ts`, `tests/services/github-service.test.ts` (feat-014)
|
||||
- `src/logic/sync-manager.ts`, `src/ui/SyncStatusView.ts`, `tests/logic/sync-manager-batch.test.ts`, `tests/ui/SyncStatusView.test.ts` (feat-015)
|
||||
|
||||
## Evidence of Completion
|
||||
|
||||
- [x] Tests pass: `npx vitest run` → 306/306 passed (post-merge, this session)
|
||||
- [x] Type check clean: `npm run build` (tsc + Obsidian 1.11.0 compat typecheck + esbuild) → clean
|
||||
- [x] Tests pass: `npx vitest run` → 344/344 passed
|
||||
- [x] Type check clean: `npm run build` → clean
|
||||
- [x] Lint clean: `npx eslint .` → 0 errors
|
||||
- [ ] Manual verification in Obsidian: in progress — user is re-testing the delete fix live; not yet confirmed resolved.
|
||||
- [x] Live smoke tests against a real GitHub repo (feat-014 push/delete, feat-015 empty-file diagnosis)
|
||||
- [ ] Manual verification inside the actual Obsidian plugin UI — not yet done
|
||||
- [ ] Pushed to remote — not yet done, needs confirmation
|
||||
|
||||
## 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.
|
||||
- Working branch: `claude/fix-directory-symlink-pull-260713` (PR #51). Commits `114a575` and `7676325` are local only — push before starting new work, or confirm with the user.
|
||||
|
|
|
|||
|
|
@ -9,52 +9,53 @@
|
|||
|
||||
## Current Objective
|
||||
|
||||
- Goal: Work through open issues one at a time, all consolidated into a single PR (user explicitly asked for fewer PRs, not one per issue). This session's issue: #38 (i18n / multi-language support).
|
||||
- Current status: #38 implemented, tested, linted, built clean, and merged onto the shared branch. 7 issues done total (#33, #36, #31, #39, #38, plus #40/#41/#42/#48 from an earlier session), all on one branch/PR.
|
||||
- Branch / commit: `claude/fix-directory-symlink-pull-260713` → **PR #51** (open). i18n work landed via a temporary branch `claude/i18n-support-260713` (commit `144eb28`), merged in and pushed.
|
||||
- Two fixes this session, both committed onto `claude/fix-directory-symlink-pull-260713` (PR #51), **not yet pushed to remote**:
|
||||
1. **feat-014** (commit `114a575`): GitHub's `pushBatch`/`deleteBatch` switched from the REST Git Data API's per-file blob-creation loop to a single GraphQL `createCommitOnBranch` mutation.
|
||||
2. **feat-015** (commit `7676325`): fixed a false "modified" status shown right after a batch push, caused by re-fetching the remote tree too soon after a write (GitHub's tree-by-branch-name read can lag a moment behind a just-completed commit).
|
||||
|
||||
## Completed This Session
|
||||
|
||||
- [x] #38 - i18n / multi-language support: `src/i18n/index.ts` (`t(key, vars?)` flat-dict lookup, `{placeholder}` interpolation, English fallback), `src/i18n/locales/en.ts` (159 keys, source of truth) and `zh-tw.ts` (full parity, verified no missing/extra keys)
|
||||
- [x] Locale detection via `window.moment.locale()`; unresolvable/unsupported locales fall back to `en`; bare `zh` maps to `zh-tw`
|
||||
- [x] Replaced ~130 hardcoded strings across `src/settings.ts`, `src/main.ts`, `src/ui/SyncStatusView.ts`, `SyncConflictModal.ts`, `WhatsNewModal.ts`, `ConfirmModal.ts`, `components/ActionBar.ts`, `FileListItem.ts`, `DiffPanel.ts`
|
||||
- [x] Left untranslated on purpose: GitHub/GitLab/Gitea proper nouns, diff-format markers (`--- Remote`/`+++ Local`), URL placeholders, changelog release-note content
|
||||
- [x] Fixed a lint regression (cognitive-complexity + nested-ternary) introduced by the change in `SyncStatusView.runBatchOperation` by extracting a lookup table and a helper method
|
||||
- [x] Added `tests/i18n/index.test.ts` (6 cases: fallback with no `window.moment`, fallback for unsupported locale, zh-tw resolution, bare `zh` → `zh-tw` mapping, interpolation, fallback-per-key when zh-tw is missing a translation)
|
||||
- [x] Worked on a temporary branch (`claude/i18n-support-260713`, off `origin/claude/fix-directory-symlink-pull-260713`) because that branch was already checked out in this repo's other worktree; merged it back into `claude/fix-directory-symlink-pull-260713` immediately afterward per the one-PR policy, rather than leaving it as a standing separate branch/PR
|
||||
- [x] Resolved a merge conflict in `feature_list.json`/`progress.md`/`session-handoff.md` against a parallel session's harness-state commit that had (incorrectly, since it predated this session's actual implementation) recorded #38 as "blocked, awaiting scope decision"
|
||||
- [x] `githubGraphQL()` helper in `github-service.ts`; explicit handling for GraphQL's 200-status-with-`errors`-array failure mode.
|
||||
- [x] `BaseGitService.getLatestCommitSha()` extracted from `resolveGitHubStyleBaseTree()`.
|
||||
- [x] `SyncManager.pushAllFiles()` now returns `syncedPaths: Array<{path, sha?}>`, populated at all three push-success sites (batch chunk, sequential fallback, immediate symlink/rename push).
|
||||
- [x] `SyncStatusView.executeBatchOperation()` marks just-pushed paths `'synced'` directly via a new `applyOptimisticSyncedStatus()` instead of calling `refreshAllStatuses()` after a push. Pull is unchanged (still does a full refresh).
|
||||
- [x] Live-verified both fixes against a real GitHub repo (`firstsun-dev/obsidian-sync-test`) using a scratchpad script that bundles the actual `github-service.ts` with a thin `obsidian` stub (`requestUrl` backed by real `fetch`) — not just mocks.
|
||||
- [x] Filed issue #57 on `firstsun-dev/git-files-sync` (Project #6, P2, 4h): build a repeatable live-credential smoke test process across GitHub/GitLab/Gitea (this session only covered GitHub).
|
||||
- [x] Cleaned up harness state: archived feat-011/012/013 + issue #78 fix + status-badge UI work into `archive/2026-07.md` (progress.md had grown past its own 80-line cleanup threshold); trimmed `feature_list.json` evidence strings under 300 chars. Harness self-audit went from 96/100 to 100/100.
|
||||
|
||||
## Verification Evidence
|
||||
|
||||
| Check | Command | Result | Notes |
|
||||
|---|---|---|---|
|
||||
| Lint | `npx eslint .` | 0 errors | Repo-wide, no exceptions |
|
||||
| Type check + compat | `npm run build` | Pass | Includes `typecheck-compat.mjs` against Obsidian 1.11.0 |
|
||||
| Tests | `npx vitest run` | 308/308 passed | 23 test files (302 pre-existing + 6 new i18n tests) |
|
||||
| Lint | `npx eslint .` | 0 errors | |
|
||||
| Type check + compat | `npm run build` | Pass | Includes Obsidian 1.11.0 compat typecheck |
|
||||
| Tests | `npx vitest run` | 344/344 passed | +3 new: 2 for `syncedPaths`, 1 more for the post-push status behavior (total 5 new across both fixes) |
|
||||
| Live smoke test | ad-hoc scratchpad scripts | Pass | feat-014: pushBatch/deleteBatch each produced one commit with correct content. feat-015: single empty file and two-identical-empty-file batches both wrote correct 0-byte blobs with matching git shas — confirmed the bug was read-side (tree fetch timing), not write-side |
|
||||
| Manual (in Obsidian) | — | Not done | No Obsidian instance available in this environment |
|
||||
|
||||
## Files Changed (this session)
|
||||
|
||||
- New: `src/i18n/index.ts`, `src/i18n/locales/en.ts`, `src/i18n/locales/zh-tw.ts`, `tests/i18n/index.test.ts`
|
||||
- Modified: `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`, `src/ui/components/FileListItem.ts`, `src/ui/components/DiffPanel.ts`
|
||||
- feat-014: `src/services/github-service.ts`, `src/services/git-service-base.ts`, `tests/services/github-service.test.ts`
|
||||
- feat-015: `src/logic/sync-manager.ts`, `src/ui/SyncStatusView.ts`, `tests/logic/sync-manager-batch.test.ts`, `tests/ui/SyncStatusView.test.ts`
|
||||
- Harness state: `progress.md`, `session-handoff.md`, `feature_list.json`, `archive/2026-07.md`
|
||||
|
||||
## Decisions Made
|
||||
|
||||
- **One PR, not one-per-issue**: user said "不要那麼多pr merge" (don't want so many PRs). All further issue work goes directly onto `claude/fix-directory-symlink-pull-260713` — do not create a new branch/PR for the next issue unless a branch conflict (as above) forces a temporary one, and merge it back in immediately if so.
|
||||
- **#38 scope**: settings.ts + all Notice() messages, done in one pass — user confirmed flat key-value dict (not nested namespaces) and `window.moment.locale()` detection with English fallback.
|
||||
- **`src/changelog.ts` is hand-curated, separate from `CHANGELOG.md`**: the auto-generated changelog (via semantic-release) isn't shipped in release assets and is too commit-log-granular for an end-user popup anyway.
|
||||
- Deduped two accidental code-duplication regressions in an earlier part of this session (test mocks, `getBlob` bodies) before they could trip SonarCloud's new-code gate again.
|
||||
- **GraphQL only for GitHub**: GitLab's Commits API already sends a whole batch in one call; Gitea has no GraphQL API. `GitServiceInterface` stays REST-based elsewhere.
|
||||
- **Optimistic local status update over re-fetching (feat-015)**: use data already known from the push itself rather than trusting an immediate remote read, which sidesteps GitHub's eventual-consistency window rather than just narrowing it with a delay/retry.
|
||||
- **Credential handling**: PATs must go into a scratchpad file the agent reads directly (`fs.readFileSync`), never typed as a `!`-prefixed command (still lands in the transcript) or passed as a Bash command-line argument (blocked by the permission classifier as credential materialization). Filed issue #57 to formalize this as the only documented path for future provider testing.
|
||||
|
||||
## Blockers / Risks
|
||||
|
||||
- None currently. PR #51 is now fairly large (7 issues). Consider flagging to the user that it may be worth reviewing/merging before more commits pile on.
|
||||
- **Commits `114a575` and `7676325` are local only** — not pushed to `origin/claude/fix-directory-symlink-pull-260713` yet. Confirm with the user before pushing.
|
||||
- PR #51 keeps growing (10+ issues' worth now) — still worth flagging before adding more.
|
||||
|
||||
## Next Session Startup
|
||||
|
||||
1. Read `CLAUDE.md`, `feature_list.json`, `progress.md`, then this file.
|
||||
2. Run `./init.sh` before editing.
|
||||
3. All new commits go on `claude/fix-directory-symlink-pull-260713` (PR #51) unless told otherwise.
|
||||
3. Check whether `114a575`/`7676325` have been pushed yet (`git log origin/claude/fix-directory-symlink-pull-260713..HEAD`); push first if not, after confirming with the user.
|
||||
|
||||
## Recommended Next Step
|
||||
|
||||
- Move to issue #37 (Bitbucket provider support) per the previously agreed order (39→38→37, all now done ahead of it).
|
||||
- Push this session's two commits, then either re-sync `gh issue list` or move to issue #37 (Bitbucket provider support) per the previously agreed order.
|
||||
|
|
|
|||
Loading…
Reference in a new issue