fix(source-control): remove stale legacy architecture references - #148
Closed
ClaudiaFang wants to merge 4 commits into
Closed
ClaudiaFang wants to merge 4 commits into
ClaudiaFang wants to merge 4 commits into
Conversation
CLAUDE.md still described src/ui/SyncStatusView.ts as the plugin's main UI and never mentioned the Source Control surface that replaced it, so an agent reading it cold would look for a file that no longer exists and miss the real call chain (SourceControlItemView -> SourceControlView -> SourceControlActionService -> SyncWorkspace -> SyncManager/executors). Also documents the two compatibility identifiers (SOURCE_CONTROL_VIEW_TYPE = 'sync-status-view', the open-sync-status command id) as intentional, not leftover legacy code to clean up.
The remote-delete E2E called service.deleteFile() directly, with a comment saying it reproduced src/ui/SyncStatusView.ts's real call path -- but that view was removed. The production path is now SourceControlActionService.deleteRemote() -> SyncWorkspace.deleteRemote() -> RemoteDeleteExecutor -> gitService.deleteFile(), which also clears tracked metadata and the live status row as part of the same call, not as a separate manual step the way this test's old manager.clearMetadata() call implied. Rebuilds the test on a real SyncManagerWorkspace + SourceControlActionService, verified against a live Gitea sandbox (npm run test:e2e -- --provider gitea: 36 passed, 18 skipped).
docs/source-control-refactor/{roadmap,phase-1..4}.md describe an
in-progress migration (roadmap.md dated 2026-08-22, still narrating
uncommitted WIP) that has since landed on main in full -- nothing in that
directory reflects the current implementation, but nothing marked it as
historical either. Adds a banner to each pointing at the new
docs/source-control.md, which describes only the current architecture and
call chain without duplicating the old roadmap's narrative.
Two regression guards so a future refactor can't silently undo this cleanup: eslint.config.mts's no-restricted-imports rule blocking ui/sync-status and SyncStatusView imports is now asserted directly (it existed before this PR but had no test locking it in), and the remote delete E2E is now locked to keep going through SourceControlActionService/SyncWorkspace rather than quietly reverting to a direct service.deleteFile() provider bypass.
|
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
Legacy cleanup only — no explicit-sync-intent changes bundled in (that's a separate follow-up PR).
CLAUDE.mdstill describedsrc/ui/SyncStatusView.tsas the plugin's main UI, which doesn't exist anymore; it now documents the real call chain (SourceControlItemView→SourceControlView→SourceControlActionService→SyncWorkspace→SyncManager/executors) and calls outSOURCE_CONTROL_VIEW_TYPE/open-sync-statusas intentional compatibility IDs, not leftovers.service.deleteFile()directly with a comment claiming it reproduced the (now-removed)SyncStatusView's real path. Rebuilt it on a realSyncManagerWorkspace+SourceControlActionService, exercisingSourceControlActionService.deleteRemote() → SyncWorkspace.deleteRemote() → RemoteDeleteExecutor → gitService.deleteFile()— the actual production chain, including the metadata/status cleanup that chain does as part of the same call.docs/source-control-refactor/{roadmap,phase-1..4}.mddescribe an in-progress migration (roadmap.md still narrated uncommitted WIP as of 2026-08-22) that has since fully landed onmain. Each now carries a historical banner pointing at a newdocs/source-control.md, which documents only the current architecture.tests/ci-workflow.test.ts: one locks in the existingeslint.config.mtsno-restricted-importsrule blockingui/sync-status/SyncStatusViewimports (the rule already existed; it had no test), the other locks the remote-delete E2E to keep going through the application layer instead of a direct provider bypass.Deliberately not touched (per the audit):
SOURCE_CONTROL_VIEW_TYPE = 'sync-status-view'andopen-sync-status(compatibility contract, not dead code), and thessv-*CSS/class prefix onDiffPanel/DiffViewer/SyncConflictModal/SourceControlView(actively used, renaming is unrelated churn).Test plan
npx eslint .— 0 errorsnpm run build— passes, includes Obsidian 1.11.0 compat typechecknpx vitest run— 864/864 passingnpm run test:e2e -- --provider gitea— 36 passed, 18 skipped, including the rebuilt remote-delete test against a live Gitea sandbox🤖 Generated with Claude Code