fix(source-control): correct status grouping and filter semantics - #132
Merged
ClaudiaFang merged 22 commits intoAug 22, 2026
Merged
Conversation
Introduce a presentation layer so the UI no longer reads Git status
directly: SourceControlSummary is the single source for every count
(all/changes/remote-changes/ready-to-push/conflicts/synced) and the
ViewModel only forwards its counts.
Filter semantics:
- all = actionable (kind !== 'synced') so All no longer duplicates the
Synced bucket.
- changes = local-side only (local-only/local-modified/moved).
- ready-to-push excludes synced.
Rendering:
- Every filter (including All) renders one flat tree + an active-filter
header; the old section breakdown under All is removed, so SYNCED never
leaks into All.
- Synced hidden by default behind a Show synced toggle; the synced chip
only surfaces when opted in, and hiding it while on synced falls back
to All.
Tree grouping:
- ChangeTreeBuilder gains TreeDisplayOptions { maxDepth,
collapseSingleChild }; the view enables collapseSingleChild so
single-child folder chains collapse to one path node instead of an
Explorer-like deep nest.
ChangeSection.ts deleted (no longer used). i18n keys + styles added.
Verification: npx eslint . 0 errors; npm run build PASS (tsc + Obsidian
1.11.0 compat + esbuild); npx vitest run 56 files / 547 tests. Manual
Obsidian verification in a real vault remains.
Scope excludes diff viewer, conflict resolution UI, push/pull pipeline,
and view migration per the fix plan's后续順序.
Add e2e/support/sync-manager-fixture.ts (real-provider service + verifier + TFile shim + auto-confirming plan/conflict modals, steered by a per-test conflict resolver) and e2e/support/source-control- scenarios.ts (high-level seed/modify/assert verbs + the Source Control selection stack wiring), so workflow suites read as seed -> modify -> push -> expect instead of 50 lines of setup per test. Add a non-breaking removeLocal to FakeVault for delete-local conflict scenarios. No production code touched; existing provider/SyncManager E2E unchanged.
Add e2e/suites/source-control-flows.e2e.test.ts and wire it into scripts/run-e2e.sh. Phase 2 covers: rename+modify (one commit, metadata moved to the new path, old path metadata cleared), multi-rename+modify batch (two moves in one commit), and the Extended nested-directory move and A->B->C rename-chain collapse (GitHub only, since they exercise SyncManager rename tracking rather than provider APIs).
Phase 3 locks the current SyncPlanner conflict contract: modify/modify with a stored baseline IS a conflict (asserted with strengthened side-effect checks — both sides + baseline + HEAD untouched on skip); delete/modify (unrelated push leaves the modified remote intact, metadata not advanced), modify/delete (blind re-create from local), rename with a remotely-edited source (move drops the old-path edit), and no-baseline add/add (local overwrites remote) are NOT conflicts today and are locked as such, so a future change to surface them as conflicts is an intentional, test-updating decision. No production behavior changed.
Phase 4 verifies the end-to-end resolution paths: keep-local pushes local content over the remote in one commit and advances metadata to the new sha; keep-remote pulls the remote blob into the vault (no remote mutation, no new commit) and updates metadata; skip is retained as a regression lock confirming local, remote, baseline metadata, and HEAD are all untouched.
Phase 5: create+modify+rename in one commit (GitHub only), a full create+modify+pure-rename+rename-with-modify lifecycle batch in one commit (all providers), and a safe+conflict batch that locks the current non-atomic contract — safe files land in one commit while the conflict is skipped and the remote stays on the remote side.
Phase 6 drives the real SourceControlActionService + PushSelectionStore + ChangeRepository over the real SyncManager (via BoundarySyncWorkspace): selected-subset push leaves unselected files untouched (core); subset- then-remaining push yields two separate commits (GitHub only); and a rename yields a path-derived ChangeId so selecting the new path's change pushes the move (GitHub only), locking the current status-model assumption. Add listCommitShas to the scenario helper.
Phase 7: remote-ahead pull advances metadata (core); a remote-ahead change and an unrelated local change coexist without cross-contamination (GitHub); a concurrent remote write surfaces as a conflict then reconciles with no lost update (GitHub); an all-unchanged batch reports zero work and zero commits (core); repeating a push makes no second mutation and corrupts no metadata (GitHub); and a skipped conflict can be resolved then re-synced cleanly with no stale operation state (GitHub).
Phase 8: unicode filename create+modify+rename, spaces-and-symbols create+modify, deeply-nested move+modify (GitHub only); 100-file batch create in one commit and a 100-file mixed modify+create+rename batch in one commit (GitHub only); plus an opt-in 1000-file stress create behind E2E_STRESS=1 (never a required CI check).
The provider-e2e job hard-coded the vitest suite list and omitted
source-control-flows.e2e.test.ts, so the new suite never ran in CI
(a "fake green" — the job passed without exercising the new coverage).
Make scripts/e2e-suites.txt the single source of truth: scripts/run-e2e.sh
reads it, expands ${provider}, and runs the listed suites; CI now calls
scripts/run-e2e.sh --provider <provider> (same command local dev uses),
collapsing the separate provision/seed/vitest/verify steps into the one
retry-wrapped entry point. Adding a shared suite now only requires editing
scripts/e2e-suites.txt.
Also make the Gitea-disabled state explicit: the gate step emits a notice
and a step-summary ("Gitea E2E: disabled — runner Docker networking") so a
green gitea leg is never mistaken for three-provider coverage.
Add scripts/check-e2e-suite-registration.mjs and wire it into `npm run lint`
(both the husky pre-commit hook and CI). It fails when an
e2e/suites/*.e2e.test.ts file exists but isn't registered in
scripts/e2e-suites.txt — provider-specific suites (github/gitlab/gitea) are
covered by the ${provider} line; every other shared suite must be listed
explicitly. So adding a suite without wiring CI now breaks the build instead
of silently passing (the original fake-green failure mode).
Replace the standalone check-e2e-suite-registration.mjs (wired into `npm run lint`) with forward/reverse checks inside scripts/run-e2e.sh itself, so suite manifest validation lives in the same script CI already calls instead of a separate Node checker. Also harden GitVerifier.git() to surface stderr on unexpected git failures while keeping expected missing-path lookups silent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
vitest's default reporter only prints once a whole file finishes, and these suites do real network round trips per test — in CI that reads as a silent hang. Switch to the verbose reporter so each test prints as it completes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
headAfterFirst was captured before the first push instead of after, so expectNoCommitSince compared against the pre-push head — failing on the commit the first push itself legitimately created, not on any duplicate mutation from the second push. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Each remote-read verifier call (getFile/fileMissing/listCommitShas) does its own `git fetch origin <branch>` even when nothing has mutated the remote since the last read in the same test — most tests do 3-5 such reads per push. Cache them in SourceControlScenario, invalidated on any call to manager.pushFiles/pullFile or service.pushFile/deleteFile. The invalidation hooks onto the manager/service instances themselves (via a thin Proxy), not this class's own push()/baseline() wrappers, so it stays correct even for mutations this class doesn't mediate directly — e.g. the selection stack's `actionService.push()`, which calls manager.pushFiles through BoundarySyncWorkspace. Scoped to source-control-flows.e2e.test.ts only (the sole consumer of SourceControlScenario); the shared GitVerifier and the other real-provider suites are untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Missing i18n keys (settings.releaseHistory.name/desc/button) referenced by settings-implementation.ts's renderReleaseHistorySetting broke npm run build. Add them to all three locales. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
My earlier fix and 8228a04 (concurrent push) both added settings.releaseHistory.{name,desc,button} independently, so the rebase merged in two copies of each key per locale file — TS1117 (duplicate object literal property). Keep 8228a04's wording, drop mine. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…erage test: expand source control real-provider E2E coverage
|
ClaudiaFang
merged commit Aug 22, 2026
9333b77
into
claude/source-control-foundation
15 of 19 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the Source Control status-grouping / filter-semantics fix. Introduces a presentation layer so the UI no longer reads Git status directly:
Changes
Phase 1 + 3 — Presentation model & single count source
SourceControlSummary.ts: pure projection + the single source for every count (all/changes/remote-changes/ready-to-push/conflicts/synced). Buckets are disjoint/exhaustive;allis actionable (excludes synced).counts.syncedis0whenshowSynced=falsewhile the raw bucket is preserved.SourceControlViewModelforwards the summary's counts instead of recomputing them, so the filter menu, headers, and tree can't drift.Phase 2 — Filter semantics
all= actionable (kind !== 'synced') — All no longer duplicates the Synced bucket.changes= local-side only (local-only / local-modified / moved).ready-to-pushexcludes synced.Phase 4 — Rendering
ALL (132)). The old section breakdown under All is removed, so SYNCED never leaks into All.syncedchip only surfaces when opted in, and hiding it while on the synced filter falls back to All.Phase 5 — Tree grouping
ChangeTreeBuildergainsTreeDisplayOptions { maxDepth, collapseSingleChild }; the view enablescollapseSingleChildso single-child folder chains collapse to one path node (02_Areas/blog/_pixnet/zh-tw/tech) instead of an Explorer-like deep nest.ChangeSection.tsdeleted (no longer used). i18n keys + styles added (en/zh-tw/zh-cn).Phase 6 — Tests
New
SourceControlSummary.test.tswith the spec cases:all = 132,synced = 36showSynced=false→ rendered synced count0≠ raw36Plus collapse-single-child tree tests; existing ViewModel/FilterMenu/SourceControlView tests updated for the new semantics.
Verification
```text
npx eslint . -> 0 errors
npm run build -> PASS (tsc + Obsidian 1.11.0 compat + esbuild)
npx vitest run -> 56 files / 547 tests
```
Manual Obsidian verification in a real vault remains (DoD for the UI surface).
Out of scope (per plan's後續順序)
Diff viewer, conflict resolution UI, push/pull pipeline, view migration. Planned order: (1) this status-model fix, (2) toolbar/select UX, (3) diff viewer, (4) conflict workflow, (5) final cleanup.
Merge flow
```
claude/source-control-foundation → fix/source-control-status-model → PR → main
```