Fix #74: drain-scoped import cancellation #83

Merged
crueber merged 1 commit from fix/issue-74 into main 2026-09-05 00:53:52 +00:00
Owner

Fixes #74 (codex major + omp major-13): the repo-import leader ran on context.WithoutCancel(context.Background()), so drain could not cancel it and it could commit a manifest CAS after drain began.

What changed:

  • Service owns a drain ctx: drive derives its Tasks().Run wait from it (no WithoutCancel remains in the package); Begin refuses fast with 503 once drained; runImport refuses the manifest PutCreate after drain begins (service flag first, body ctx second — a post-drain commit fails, never lands).
  • serve.go phase 1 drains both halves: reg.Tasks().Drain() (kills the clone via CommandContext, fails store work) + importSvc.Drain() (cancels the leader wait).
  • Same change fixes a latent wal.TaskTable race the drain path exposed: Run assigned rt.cancel after mu.Unlock while Drain reads it; cancel is now published with the map entry (join/refuse paths release their unused ctx).
  • Regression tests in internal/repoimport/drain_test.go: hanging-clone import drained to a prompt 503 terminal with no manifest committed; Begin-after-drain 503; drained headless run refuses at the commit point.
  • Doc Decisions entry in docs/features/10_git_import.md (law 12).

Verification: go test -race ./internal/repoimport/ ok, coverage 95.6% (>=95%); go test -race ./internal/wal/ ok (incl. -count=5); ./cmd/walhub ok; gofmt/vet clean. Scratch worktree from origin/main; main worktree untouched.

Fixes #74 (codex major + omp major-13): the repo-import leader ran on context.WithoutCancel(context.Background()), so drain could not cancel it and it could commit a manifest CAS after drain began. What changed: - Service owns a drain ctx: drive derives its Tasks().Run wait from it (no WithoutCancel remains in the package); Begin refuses fast with 503 once drained; runImport refuses the manifest PutCreate after drain begins (service flag first, body ctx second — a post-drain commit fails, never lands). - serve.go phase 1 drains both halves: reg.Tasks().Drain() (kills the clone via CommandContext, fails store work) + importSvc.Drain() (cancels the leader wait). - Same change fixes a latent wal.TaskTable race the drain path exposed: Run assigned rt.cancel after mu.Unlock while Drain reads it; cancel is now published with the map entry (join/refuse paths release their unused ctx). - Regression tests in internal/repoimport/drain_test.go: hanging-clone import drained to a prompt 503 terminal with no manifest committed; Begin-after-drain 503; drained headless run refuses at the commit point. - Doc Decisions entry in docs/features/10_git_import.md (law 12). Verification: go test -race ./internal/repoimport/ ok, coverage 95.6% (>=95%); go test -race ./internal/wal/ ok (incl. -count=5); ./cmd/walhub ok; gofmt/vet clean. Scratch worktree from origin/main; main worktree untouched.
drive() ran on context.WithoutCancel(context.Background()): drain
could not cancel an in-flight import and the leader could commit a
manifest CAS after drain began (13 phase-1 ctx rule, law 7).

- Service owns a drain ctx: drive derives its Tasks().Run wait from
  it; Begin refuses fast with 503 once drained; runImport refuses
  the manifest PutCreate after drain begins (flag first, body ctx
  second). No WithoutCancel remains in the package.
- serve.go phase 1 drains both halves: reg.Tasks().Drain() kills
  the clone + store work, importSvc.Drain() cancels the leader.
- wal.TaskTable.Run published rt.cancel after mu.Unlock while Drain
  reads it: cancel is now installed with the map entry (join/refuse
  paths release their unused ctx).
- Regression: internal/repoimport/drain_test.go (hanging clone,
  Begin-after-drain, commit-point guard). Doc Decisions entry in 10.
Sign in to join this conversation.
No description provided.