Pre-flight repo-sync gate on structural writes, with a copyable divergence report
agent:in-progress
## Problem Statement
When the database's record of a Course's structure and the on-disk CourseRepo disagree — a real Lesson or Section the DB tracks has no directory on disk, or a numbered directory exists on disk that the DB doesn't track — any structural edit becomes unrecoverable. The move/rename/reorder cascade plans `git mv`/`git rm` operations assuming disk matches the DB, so it fires those operations against paths that don't exist and dies mid-cascade with a cryptic `fatal: renaming '...' failed: No such file or directory`. Retrying never helps, because the bad state lives in the data, not in a transient glitch. Worse, a mid-cascade failure can leave disk half-mutated against the DB, *creating* fresh divergence.
The user (the course author, who edits structure by dragging lessons and using context menus in the course view) sees only "Action failed — your change was reverted." The actual diagnosis — which directory is missing, which is orphaned — exists but is buried in server logs. There is no way to get a clear, actionable description of the divergence into the hands of an agent that could reconcile it.
## Solution
Refuse to act on a divergent repo, and when we refuse, hand the user a precise, copyable report they can paste straight into an agent to fix.
Before any filesystem-mutating write, run the existing repo-sync validation as a **pre-flight gate**. If the DB and disk disagree, the write fails immediately — before touching anything — with the full mismatch report (every missing directory and every orphan directory, with expected paths). Because the gate runs before mutation, a failed write is a clean no-op: nothing is half-applied, so no rollback machinery is needed.
When that failure reaches the course view, instead of a generic toast the user gets a modal showing the verbatim report with a Copy button and a one-line "repo is out of sync — hand this to an agent to reconcile" framing. The optimistic change still reverts underneath, exactly as today.
## User Stories
1. As a course author, when I drag a lesson into another section while my repo's DB and disk disagree, I want the move to be refused before anything changes, so that the move never half-applies and creates more divergence.
2. As a course author, when a structural edit is refused for divergence, I want to see exactly which Section or Lesson directories are missing on disk, so that I know what is wrong.
3. As a course author, I want to see exactly which on-disk directories are orphaned (present on disk but untracked in the DB), so that I can pair them with the missing ones and understand what moved where.
4. As a course author, I want the divergence report in a modal I can read in full without truncation, so that a long list of mismatches is not cut off.
5. As a course author, I want a Copy button on the divergence report, so that I can paste it into an agent in one action.
6. As a course author, I want a one-line explanation that the repo is out of sync and should be handed to an agent to reconcile, so that I know what to do next.
7. As a course author, I want my optimistic change to revert when a write is refused, so that the UI never shows a change that did not happen.
8. As a course author, I want divergence to be caught not just on lesson moves but on lesson delete, lesson rename, lesson reorder, convert-to-ghost, section reorder, and section rename, so that no structural edit can act on a divergent repo.
9. As a course author, I want materializing a ghost lesson or creating a real lesson to also refuse on a divergent repo, so that materialization never builds on a broken on-disk state.
10. As a course author, I want purely in-database edits (adding a ghost section, adding a ghost lesson, archiving a section) to remain unaffected, so that planning actions that never touch disk stay fast.
11. As a course author, I want a ghost-only edit (renaming or moving a ghost lesson that has no directory) to remain fast, so that I do not pay a disk scan for an edit that never touches disk.
12. As a course author, when my repo is in sync, I want my edits to keep working exactly as before, so that the gate is invisible in the normal case.
13. As a course author, when a write succeeds, I want the existing post-write validation to keep running, so that a logic bug that introduces divergence is still caught immediately rather than only on my next edit.
14. As a course author, when an edit fails for a reason that is not divergence (e.g. a generic server error), I want the existing generic failure toast, so that the divergence modal is reserved for divergence.
15. As an agent reconciling a divergent repo, I want a report that names both the missing and the orphaned directories, so that I can infer the corrective `git mv` and fix the divergence in one pass.
16. As a developer reading the validation helpers later, I want the code comment to accurately describe that pre-flight validation runs as a deliberate gate, so that I am not misled into thinking pre-validation was removed.
## Implementation Decisions
- **Reuse the existing validator.** The pre-flight gate calls the existing `CourseRepoSyncValidationService.validate({ repoPath })` — the same repo-scoped scan currently used for post-write validation. It already reports both missing tracked directories and untracked orphan directories with expected paths, and already derives Section real-ness from lessons (never the path prefix), consistent with the existing section real-ness rule. No new validation logic, no new error type — it raises the existing `CourseRepoSyncError`.
- **Gate placement: `CourseWriteService`.** The gate is added inside `CourseWriteService`, the single service through which every structural write flows. For the always-filesystem operations (materialize ghost, create real lesson, materialize course-with-lesson, reorder lessons, reorder sections, rename section), validation runs before the operation as well as after. For the conditionally-filesystem operations (move lesson to section, delete lesson, rename lesson, convert to ghost), the pre-flight runs under the *same* condition each already uses to decide whether it touches disk — e.g. a lesson move only validates when the plan emits filesystem operations; a lesson delete/rename only validates for a real (non-ghost) lesson. A ghost-only edit pays nothing.
- **Pre + post, not pre-only.** Both a pre-flight and the existing post-write validation run on filesystem-touching writes. Pre-flight refuses to act on an already-divergent repo; post-write still catches divergence a write's own logic might introduce, at the operation that caused it. This accepts two full repo scans per filesystem write as the cost of both guarantees.
- **DB-only operations are untouched.** Adding a ghost section, adding a ghost lesson, and archiving a section never touch disk and get no gate.
- **No rollback / atomicity machinery.** Because the gate runs before any mutation, a divergence failure is inherently a clean no-op. Compensating-operation rollback for genuine mid-cascade filesystem failures (disk full, permission change, concurrent process) is explicitly not built here.
- **Pure planner unchanged.** The lesson-move planner stays a pure data-in/data-out function and is not modified; the shared-planner architecture (server executes, client replays optimistically) is preserved.
- **Client surfacing at the fetcher-watch layer.** The course view's optimistic hook, which already watches course-editor fetchers and shows a generic failure toast on a non-ok response, gains a branch: when a course-editor fetcher returns a `409` carrying a `CourseRepoSyncError`, it reads the response body text and opens a modal dialog (matching the app's existing modal pattern) showing the verbatim report, a Copy-to-clipboard button, and the "out of sync — hand to an agent" framing. All other failures keep the existing generic toast. The optimistic revert behavior is unchanged.
- **API contract unchanged.** The course-editor action already maps `CourseRepoSyncError` to HTTP 409 and serializes its `message` into the response body. No new status codes or payload shape; the client simply starts reading the body it already receives.
- **Honest comment.** The validation-helpers comment that currently states pre-validation was removed for performance is rewritten to describe the deliberate pre-flight gate and the accepted double-scan cost.
## Testing Decisions
- **Test external behavior, not internals.** Tests assert observable outcomes: a write against a divergent repo fails with `CourseRepoSyncError`, and both the database and the on-disk repository are left exactly as they were before the call (no half-applied directory renames, no partial DB updates). Tests against an in-sync repo assert the write still succeeds and produces the same result as today.
- **Tested at one seam: `CourseWriteService`.** Tests drive the service against a real pglite database and a real temporary git repository, the established pattern in `course-write-move.test.ts`. A divergent fixture is built by recording a real Section/Lesson in the DB whose directory is absent on disk (and/or an on-disk numbered directory the DB does not track), then invoking each filesystem-touching write and asserting clean refusal. A ghost-only edit is asserted to bypass the gate (succeeds without a disk scan affecting it). In-sync happy-path coverage already exists and must stay green.
- **Prior art.** `course-write-move.test.ts` (service + real DB + real temp git repo, including a noted `CourseRepoSyncError`-or-post-validation failure path in the e2e variant), `course-repo-write-section-delete-rename.test.ts` (real temp git repo manipulation), and `course-write-e2e.test.ts`. Tests use vitest with the project's Effect layer-provisioning helpers.
- **Client modal not unit-tested.** The modal trigger is a thin view concern at the optimistic hook's existing fetcher-watch effect, which has no current unit coverage; it is verified manually rather than by introducing a new React-hook test seam, keeping the tested-seam count at one.
## Out of Scope
- Rollback / compensating-operation atomicity for genuine mid-cascade filesystem failures (disk full, permission flip, concurrent mutation). The gate prevents acting on existing divergence and prevents the divergence-driven half-apply; it does not make an arbitrary fs failure transactional.
- Automatic reconciliation. The modal surfaces and copies the report; it does not fire a fix to an agent or run corrective `git mv` itself.
- Committing staged git changes. The app continues to only stage filesystem operations; introducing commits is unrelated.
- A cheaper scoped pre-flight (statting only the paths a single operation touches). Rejected in favor of the full scan because the full scan's missing+orphan pairing is what makes the report agent-fixable.
- Any change to the pure lesson-move planner or the shared-planner architecture.
- A new glossary term for the divergence concept (considered and declined).
## Further Notes
- The incident that motivated this: the DB recorded a real lesson in section `06-improving-our-flow`, but on disk that lesson lived under `07-improving-our-flow` and no `06-improving-our-flow/` directory existed. The planner, correctly treating a section with a real lesson as real, emitted `git mv` against the non-existent `06` directory, and every retry failed identically. A pre-flight gate would have refused the move and reported the missing `06` directory and the orphan under `07`.
- Performance: the validator's scan is O(sections × lessons) `fs.exists`/`readDirectory` calls and is comparatively slow on WSL 2 (~100ms per call historically). Running pre + post means two scans per filesystem write. This cost was reviewed and accepted given structural edits are user-initiated, not hot-path; if it proves painful, dropping post-validation (pre-only) is the leaner fallback.
- An ADR was considered (the change reinstates pre-validation that was previously removed for performance, a documented reversal with a real trade-off) but not requested; the rewritten validation-helpers comment is the minimum needed to keep the codebase honest about the reversal.
0 条评论