Skip to content

fix(daemon): target local scans from watcher events - #329

Merged
isac322 merged 24 commits into
mainfrom
isac322/daemon-rescans-the-whole-tree-on-every-watcher-w
Nov 3, 2026
Merged

isac322 merged 24 commits into
mainfrom
isac322/daemon-rescans-the-whole-tree-on-every-watcher-w

Conversation

@isac322

@isac322 isac322 commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

TL;DR

Summary

A busy project could spend a full scan on every local watcher wake, including writes BearDrive had just materialized. A short-lived temporary file could also stop the watcher and reduce coverage to tight polling. This PR keeps local edit latency while limiting discovery to affected paths and preserves the existing full-scan safety path.

Root cause

The watcher collapsed filesystem events into a boolean wake, so the syncer could not distinguish materialization echoes or pass changed paths to local discovery. Session.Cycle therefore walked the whole mount on each wake, while recursive watcher setup treated child ENOENT errors as fatal. See the analysis and reproduction record in #328.

Design

The daemon accumulates bounded per-mount event batches with fences, coalesces repeated paths, and passes a private scan hint to the syncer. Existing callers keep the nil-hint full-scan behavior. Targeted scans reuse the existing walk predicate and limit cache deletion to the affected path or subtree. Startup, joins, policy changes, scope changes, overflow, uncertainty, watcher loss, and the independent one-minute safety deadline force full discovery.

Post-cycle suppression compares the current filesystem state with the saved cache. It drops only proven echoes and retains mismatches, unknown paths, tracked deletions, and events arriving after the fence. Recursive watcher setup rebuilds from an independent candidate watcher when a wrapped ENOENT identifies a transient child race. Missing roots, non-ENOENT failures, closed channels, and overflow remain fatal or force polling.

Changes

  • internal/daemon: bounded per-mount event batches, targeted hint routing, echo suppression, safety deadline, watcher rebuilds, and wrapped ENOENT classification.
  • internal/syncer: nil/empty/targeted scan hints, scoped cache reconciliation, shared targeted traversal, and nested-mount ancestor protection.
  • Tests: daemon watcher lifecycle and fence coverage, syncer cache/delete/join/policy/nested-mount coverage, and the Darwin-equivalent issue reproducer.
  • Architecture: Session now exposes the internal targeted-scan entry point, ScanHint/ScanEvent model the daemon-to-syncer boundary, and volumeWorker routes watcher batches through localFolderWatcher.

QA checklist

ID Behavior Test Tier
QA-SCAN-01 Nil/default callers still perform full discovery TestCycleWithoutHintStillJournalsUnnamedEdit integration
QA-SCAN-02 Targeted scans do not delete unvisited cache entries TestTargetedScanDoesNotDeleteUnvisitedCacheKeys integration
QA-SCAN-03 Targeted deletes preserve unrelated cache entries TestTargetedScanReconcilesExactDelete integration
QA-SCAN-04 Joins ignore partial hints and policy changes force full scans TestJoiningIgnoresPartialHint, TestIgnoreEditForcesFullScan integration
QA-SCAN-05 Empty hints pull without local delete reconciliation TestEmptyHintPullsWithoutLocalDeletePass integration
QA-SCAN-06 Nested mount boundaries remain protected TestNestedMountExcluded integration
QA-WATCH-01 Events coalesce and remain per mount TestLocalFolderWatcherBatchesPerMount daemon
QA-WATCH-02 Fences retain later events and suppress proven echoes TestEventsDuringScanAreFenced, TestMaterializationEchoSuppressionKeepsChangedEvents, TestWorkerSuppressesLateMaterializationEchoes daemon
QA-WATCH-03 Transient and wrapped child ENOENT races recover TestWatcherRebuildsOnChildENOENT and async error coverage daemon
QA-WATCH-04 Remote wake and remote timer cycles capture pending local edits before pull TestWorkerRemoteWakePreservesPendingLocalEdit, TestWorkerRemoteTimerPreservesQueuedLocalChanges daemon
QA-WATCH-05 Safety deadline survives targeted/remote-only cycles, local debounce, and a remote cycle that spans the deadline TestWorkerSafetyFullScanDeadlineSurvivesOtherCycles, TestWorkerLocalWakeDebounceHonorsSafetyDeadline, TestWorkerRemoteCycleCannotExtendSafetyDeadline daemon
QA-WATCH-07 A remote-only wake receives peer bytes without discovering an unrelated unhinted local file; the file uploads once its local hint arrives TestWorkerRemoteOnlyWakeDoesNotDiscoverUnhintedLocalFile daemon
QA-WATCH-08 A recovered targeted-cycle panic resets discovery to full while retaining backoff and cancellation TestWorkerTargetedPanicForcesFullRediscovery, TestSupervisor_WorkerPanicRecoveryIsolatesOtherVolumes daemon
QA-SYNC-01 Symlink ancestors and untracked symlinks are never followed or clobbered TestTargetedLeafBelowSymlinkDoesNotJournalTargetBytes, TestEmptyHintPullPreservesUntrackedSymlinksAtPeerRegularPaths integration
QA-SYNC-02 Writable tracked deletions and link edits survive pull until a targeted scan TestEmptyHintPullPreservesWritableLocalDeletionUntilTargetedRemove, TestEmptyHintPullPreservesWritableSymlinkEditUntilTargetedWrite integration
QA-SYNC-03 Tracked regular↔symlink transitions still materialize TestEmptyHintPullMaterializesRegularToSymlinkTransition, TestEmptyHintPullMaterializesDanglingSymlinkToRegularTransition integration
QA-WATCH-06 Original Darwin scenario preserves edits and removes echo cycles TestIssue328ReproEchoCycle, TestIssue328ReproConcurrentUserWrite, TestIssue328ReproWatcherDiesOnShortLivedFile Darwin smoke
QA-PERF-01 Large-tree wake cost is measured after full cycle completion TestIssue328ReproNoopCycleCost Darwin smoke

Verification

Commands run:

  • go test ./... -count=1 -timeout=45m: passed on final source committed as a2ab967, including current main.
  • go vet ./... and go build ./...: passed on that source.
  • go test -race -timeout=30m ./internal/daemon ./internal/syncer: passed.
  • go test -race -timeout=20m ./internal/webapp -run 'Collab|Concurrent|Stress|Events|Presence|Upload|Store': passed.
  • go test -race ./internal/webapp -count=100 -timeout=10m -run '^TestYCollabAcceptsARealWebsocket$': passed after joining the original unload callback before fixture cleanup. The old cleanup reproduced four failures in 100 iterations.
  • The queued-local-change regression failed against the pre-fix daemon and passed on the fixed daemon.
  • Forcing full discovery on remote cycles makes the new remote-only regression fail by discovering the unhinted local file. The intended bounded path passes.
  • go test -race ./internal/daemon -count=10 -timeout=5m -run '^TestWorkerLocalWakeDebounceHonorsSafetyDeadline$': passed. The identical test fails against the pre-fix 4a36197 runtime because the unobserved file misses its discovery bound. Both outer polling and inner debounce now honor the safety deadline.
  • TestWorkerRemoteCycleCannotExtendSafetyDeadline failed on 547e1da with peer content received but the unobserved local file still missing from the journal after five seconds. It passes after recomputing the absolute safety deadline after the cycle, including 10 race iterations alongside the debounce test.
  • TestWorkerTargetedPanicForcesFullRediscovery failed on b2b462a with the hinted edit retried but the unhinted file undiscovered. The full-recovery reset passes, including 10 race iterations with the safety clock frozen and watcher admission blocked.

Earlier reproduction observations on 54d0b11 (the later runtime correction covers remote timer/follow-up ordering):

  • No peer changes: 2 cycles after one local edit.
  • 20 peer files: 2 cycles after one local edit, with 2 activity logs and no echo-only cycle.
  • Concurrent user edit during pull: passed and journaled.
  • Short-lived temp-file churn: watcher survived and still detected a later user write.
  • 5,000 files: 214 ms whole-process CPU for the completed no-op cycle.
  • 40,000 files: 1,451 ms whole-process CPU for the completed no-op cycle.

Old vs new (same reproducer family):

QA ID v0.18.1 (64b6af4) this branch (54d0b11) Environment
QA-WATCH-04 echo 3 cycles with 20 peer files 2 cycles with 20 peer files Darwin arm64, Go 1.26.10
QA-WATCH-03 child ENOENT watcher stopped with lstat ENOENT watcher survived churn and detected a later write Darwin arm64, Go 1.26.10
control no-peer scenario completed; concurrent user-write control passed no-peer scenario completed; concurrent user-write control passed Darwin arm64, Go 1.26.10
  • Reporter-equivalent environment check: Darwin arm64 and Go 1.26.10, using the issue's temporary-file and large-tree scenarios.

Architecture changes

architecture/cli-sync.md: Session now has the internal CycleWithScanHint path; ScanHint owns ScanEvent values; and volumeWorker routes batches from localFolderWatcher to targeted local discovery. The diagram update is committed with this PR.

Risks and upgrade impact

  • No persisted schema or journal format changes.
  • Existing CLI, hook, desktop, and one-shot callers retain full-scan behavior through the nil-hint path.
  • Targeted directory echo comparison can walk the affected subtree before suppressing an event; uncertain results remain queued or force a full scan.
  • The one-minute full-scan deadline remains the completeness backstop.

Not run / not changed / not added

  • Not run: Windows-specific watcher behavior. This issue concerns the Darwin kqueue path; existing cross-platform package tests passed on the current host.
  • Not changed: remote watch protocol, journal ordering, replay semantics, store schema, CLI/config flags, and unrelated remote polling behavior.
  • Pre-existing defect: peer deletion can leave a tracked dangling symlink on disk, reproduced on both base eaa6e0d and earlier PR revision 77b1871. The unchanged deletion loop is tracked separately in Peer deletion leaves tracked dangling symlinks on disk #331.
  • Not added: fixed millisecond CPU thresholds. Cost evidence is reported as relative measurements because absolute limits are machine-dependent.

Non-gating CI legs

None.

@isac322 isac322 self-assigned this Nov 2, 2026
@isac322
isac322 requested a review from runbear-bot November 2, 2026 12:47

@isac322 isac322 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GREEN

PR-ready: true
Reviewed head: 6c3c16c
CI: all required checks passed (Ubuntu, macOS, race, Postgres metadata). Mergeable: true.

Verified:

  • The targeted scan preserves nil full-scan callers and scopes cache deletion to hinted paths.
  • Watcher batches are per mount, fenced, bounded, and conservative about echo suppression.
  • Nested mount ancestors remain excluded from targeted scans.
  • Wrapped child ENOENT errors rebuild or retain watcher coverage without swallowing missing roots or other backend failures.
  • The latest race fix synchronizes test hooks and joins watcher shutdown before cleanup.
  • Darwin reproductions cover echo suppression, concurrent edits, short-lived temp-file churn, and large-tree cost.

Not verified: Windows-specific watcher behavior and a published release artifact. The required runbear-bot review is still pending.

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: not-ready

  • P2 — watcher rebuild/shutdown race: closeWatcher consumes sync.Once while closing the old watcher, but rebuildWatcher can then install a new nw. The deferred close becomes a no-op; because stopLocalWatch waits for done, shutdown can hang with the replacement fsnotify watcher still open. See the inline finding.
  • P2 — same-volume nested-mount routing: the shared recursive watcher records nested directories under the first parent root in seen. Event handling prefers seen[name] over the more-specific rootFor, so directory remove/rename events inside a nested mount can be queued to the parent. The parent targeted walk skips that nested mount, while the nested target receives no hint; its cache/journal can remain stale until the full-scan deadline. This is body-only because the defect spans the shared watcher ownership and target routing rather than one uniquely responsible changed line.

Verification

  • git diff --check: passed.
  • Go checks were blocked before discovery because the checkout environment has no go executable: go test ./internal/daemon, go test ./internal/syncer, go test ./..., go vet ./..., and go build ./... each 0 / 0 / 0 (blocked: go: command not found).
  • Independent exact-source review completed against head 6c3c16c9d2db0a233255f48c95115c307f3c83c7; no live Darwin/Windows check was run.

Comment thread internal/daemon/daemon.go Outdated
closeOnce.Do(func() {
watcherMu.Lock()
defer watcherMu.Unlock()
_ = watcher.Close()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 one-shot close 경로는 rebuildWatcher가 candidate를 만드는 동안 실행될 수 있습니다. 이 호출이 old watcher를 닫고 closeOnce를 소비한 뒤 rebuildWatcher가 watcher = nw로 교체하면, deferred closeWatcher는 no-op이 됩니다. 현재 stopLocalWatch는 done을 기다리므로 새 watcher가 계속 선택 루프를 점유해 종료가 멈추고 fsnotify descriptor도 남을 수 있습니다. close/rebuild를 하나의 종료 상태 아래 직렬화하거나, 교체 후에도 새 watcher를 반드시 닫는 경로가 필요합니다.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 41b593b and verified with the focused daemon tests, the exact race command, and the full Go/vet/build gates.

The watcher now marks shutdown under the same mutex used for candidate swaps. A candidate created after shutdown is closed instead of being installed, and the active watcher is closed exactly once before the watcher goroutine joins.

@isac322

isac322 commented Nov 2, 2026

Copy link
Copy Markdown
Member Author

Implemented both P2 findings from the current review in 41b593b.

  • Watcher rebuild and shutdown are serialized. A replacement created during shutdown is closed and cannot keep the watcher goroutine alive.
  • Events inside a same-volume nested active root now route to the most-specific root instead of the parent batch.

Verified locally:

  • Focused watcher lifecycle, nested-root routing, echo/fence tests passed.
  • go test -race -timeout 20m $(go list ./... | grep -v "/internal/webapp$") passed.
  • Full go test ./..., go vet ./..., and go build ./... are running for the current head.

@isac322
isac322 requested a review from runbear-bot November 2, 2026 14:05

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: not-ready

  • P1 — remote wake can run before a pending local batch is collected: when the hub watch wins the wait select, the worker resets nextHints and proceeds directly to the next cycle. The local watcher wake remains pending, but that cycle uses an empty hint, so local discovery is skipped while pull/materialization can overwrite a same-path local edit before it is journaled. See the inline finding.
  • P2 — the one-minute full-scan deadline is not a deadline: the worker forces a full scan only when the cycle starts after the deadline, but then waits a fresh pollEvery interval. A targeted cycle just before the deadline can therefore defer the safety scan by almost another minute. See the inline finding.

Verification

  • git diff --check: passed.
  • Go verification was blocked before discovery because this checkout has no go executable: go test ./..., focused daemon/syncer tests, go vet ./..., and go build ./... are each 0 / 0 / 0 (blocked: go: command not found).
  • Independent exact-source probes and static review completed against head 96185781da318e9a81ff03e4c4136c5bb2d718d4.
  • The earlier watcher rebuild/shutdown and same-volume nested-root findings were rechecked at this head and are not retained.

Comment thread internal/daemon/daemon.go Outdated
// A peer pushed. Clearing the gate makes the cycle below a remote
// one instead of a cheap local scan.
lastRemote = time.Time{}
nextHints = make(map[string]*syncer.ScanHint)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: 이 분기에서 nextHints를 비우고 바로 다음 사이클로 넘어가지만, pending local watcher batch를 collectLocalHints()로 수집하지 않습니다. 같은 파일의 로컬 편집 이벤트가 아직 localWatchWake에 남아 있어도 이 사이클은 빈 hint로 local discovery를 건너뛰고 pull/materialize를 수행하므로, 동시 peer 변경이 로컬 편집을 덮어쓸 수 있습니다. remote wake 전에 pending local batch를 수집하거나 full scan을 강제해야 합니다.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in b21aa54 plus the final oracle/test fixes. A remote wake now calls collectLocalHints() before the next scan/pull, preserving scan-before-pull for a local edit that raced the peer wake. TestWorkerRemoteWakePreservesPendingLocalEdit blocks the remote List, asserts the local journal/blob exists before release, then verifies peer materialization and local disk bytes.

Verified on the final head 54d0b11 with the focused daemon safety tests and the full Go/vet/build suite.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 0a398e6. The worker now merges pending watcher batches immediately before every non-full cycle, covering remote timer cycles and the local-ops follow-up while preserving the bounded empty-hint path for genuine remote-only cycles. The new TestWorkerRemoteTimerPreservesQueuedLocalChanges fails against the pre-fix daemon and passes on this head; it asserts the queued local put and delete are journaled before the blocked pull. Focused daemon/syncer tests, isolated go test ./..., vet, build, and daemon/syncer race all pass locally.

Comment thread internal/daemon/daemon.go Outdated
waitFor := pollEvery
remoteFailed := false
doRemote := proj.Remote != "" && time.Since(lastRemote) >= remotePollEvery
if !localWatchHealthy || lastFullScan.IsZero() || localWatchNow().Sub(lastFullScan) >= localWatchSafetyInterval {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: 이 체크는 이미 안전기한을 넘겼을 때만 forceFull을 세우고, 아래의 waitFor는 여전히 새 pollEvery에서 시작합니다. 마지막 full scan 직후가 아닌 59초 뒤에 targeted cycle이 실행되면 다시 거의 1분을 기다려 문서화된 one-minute safety deadline을 넘길 수 있습니다. waitFor를 lastFullScan + localWatchSafetyInterval까지의 남은 시간으로 제한해야 합니다.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in b21aa54. waitFor is now clamped to lastFullScan + localWatchSafetyInterval, including zero when the deadline is due, so targeted and remote-only cycles cannot postpone the safety scan.

TestWorkerSafetyFullScanDeadlineSurvivesOtherCycles uses a wall-progressing clock offset, no worker wake to force a full scan, blocks watcher installation, and observes the unqueued file through its journal/blob/push cursor. Mutation checks removing the clamp fail this regression. Verified on final head 54d0b11 with focused daemon tests and the full Go/vet/build suite.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented the remaining debounce path in 547e1da. The inner local-wake wait is bounded by the same safety deadline as the outer poll. When the deadline expires during debounce, forceFull is set before the echo-only early-out. TestWorkerLocalWakeDebounceHonorsSafetyDeadline fails against 4a36197, passes on the fix, and passed 10 race iterations. The full Go suite, vet, build, daemon/syncer race, and exact webapp CI race command all pass locally after merging current main.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the long-cycle case in b2b462a by moving the existing safety clamp to immediately before waitDeadline. The remaining time now uses the latest lastFullScan and localWatchNow after remote work finishes, so the cycle duration is not added again. The inner debounce clamp and expired-deadline forceFull remain in place.

TestWorkerRemoteCycleCannotExtendSafetyDeadline fails on 547e1da with the peer file received but the unobserved local file not discovered, and passes on this fix. The long-cycle and debounce tests passed 10 race iterations; full Go tests, vet, build, daemon/syncer race and the exact webapp CI race command also passed locally.

@isac322
isac322 requested a review from runbear-bot November 2, 2026 16:47

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: inspection-incomplete

검토 도중 HEAD가 변경되어 최신 커밋을 검토하지 못했습니다. 또한 이 환경에는 Go 실행 파일이 없어 관련 테스트와 재현을 실행하지 못했습니다. 최신 HEAD에서 재검토하고 Go 검증을 실행할 수 있을 때 다시 요청해 주세요.

@isac322
isac322 requested review from runbear-bot and removed request for runbear-bot November 2, 2026 17:15
@isac322
isac322 removed the request for review from runbear-bot November 2, 2026 17:28
@isac322
isac322 requested a review from runbear-bot November 2, 2026 17:28

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: ready-with-notes

이전 지적의 watcher 종료 동기화, 중첩 mount 라우팅, remote wake 시 local batch 수집 및 safety deadline 제한을 현재 소스에서 재확인했습니다. 추가로 확인된 merge blocker는 없습니다.

검증 및 한계

  • git diff --check origin/pr/329-base...HEAD: 통과.
  • Go 실행 파일이 없어 daemon/syncer 테스트와 race 검증은 실행하지 못했습니다. Darwin/Windows 실환경 검증도 수행하지 않았습니다.
  • 작성자는 이전 head의 Go 테스트 및 빌드 통과를 보고했으나, 이번 리뷰에서 독립 실행한 결과는 아닙니다.

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: ready-with-notes

현재 HEAD 77b18717e289bc40bec4d2342f3f2731445bd7f1 기준으로 merge-blocking finding은 없습니다.

  • targeted local scan의 범위 제한·cache/delete reconciliation·nested mount 보호를 확인했습니다.
  • watcher 교체/종료 race, same-volume nested-root routing, remote wake의 pending local edit, one-minute safety deadline에 대한 이전 finding은 현재 코드와 회귀 테스트에서 재현되지 않았습니다.

Verification

  • git diff --check: passed.
  • Go tests, vet, build, gofmt: 0 / 0 / 0 — blocked before discovery (go: command not found). 따라서 runtime, race-detector, Darwin/Windows watcher 실행 결과는 확인하지 못했습니다.
  • 본 리뷰는 body-only이며 changed-line inline comment는 게시하지 않습니다.

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: not-ready

  • P1 — tracked symlink deletions are not materialized: materialize handles cached paths absent from the replayed target through internal/syncer/syncer.go:2046-2090. It calls store.UnderRoot (which rejects dangling/outside-resolving symlinks) and then os.Stat (which follows symlinks) before os.Remove. A peer deletion of a tracked dangling symlink therefore drops the cache entry without unlinking the symlink; a live/outside-target symlink is likewise retained when its target metadata does not match. The next cycle has no cache entry to retry, leaving stale local state. This is body-only: the responsible deletion block is unchanged, so there is no truthful changed-line anchor. Use Lstat/link-aware removal while preserving the dirty-local-edit guard.

Verification

  • git diff --check: passed.
  • Go verification was blocked before discovery because this environment has no go executable: go test ./internal/daemon, go test ./internal/syncer, go test ./..., go vet ./..., and go build ./... each 0 / 0 / 0 (blocked: go: command not found).
  • Independent exact-head review completed against 77b18717e289bc40bec4d2342f3f2731445bd7f1; the latest commit is documentation-only. A filesystem semantics probe confirmed os.Stat fails on a dangling symlink while os.Lstat still identifies the link; repository Go tests could not run without Go.

@isac322

isac322 commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

The tracked dangling-symlink deletion finding is real but predates this PR: the same two-device reproduction fails on base eaa6e0d and PR revision 77b1871, and this PR leaves that deletion block unchanged. It is tracked separately in #331 rather than expanding the watcher fix.

The final source at 4a36197 passes the isolated full Go suite, vet, build, daemon/syncer race, and the exact webapp CI race command locally. The remote-only worker test receives peer content without discovering an unhinted local file, then uploads that file after its watcher event arrives; forcing full scans on remote cycles makes the test fail. The separate WebSocket fixture cleanup race reproduced four failures in 100 iterations and now passes 100 race iterations by joining the real unload callback before temporary storage cleanup.

@isac322
isac322 requested a review from runbear-bot November 3, 2026 02:18

@isac322 isac322 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GREEN on 4a36197. PR-ready: false until current-head CI and the requested runbear-bot review complete.

Verified locally: full Go suite, vet, build, daemon/syncer race, and the exact webapp CI race invocation. The remote-only worker test passes and fails when remote cycles force Full discovery; the queued-local-change regression fails before the runtime fix and passes afterward. The WebSocket test cleanup joins the original unload callback and passed 100 race iterations after reproducing the old cleanup failure. No blocking regression was found in the current diff. The pre-existing symlink deletion defect is tracked separately in #331.

Not verified: Windows-specific watcher execution. This account authored the PR, so this internal verdict is recorded as a comment rather than a self-approval.

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: not-ready

  • P2 — safety full-scan deadline remains bypassable during watcher debounce. The outer wait is now clamped, but a local wake enters the inner lastCycle + scanInterval timer without also observing lastFullScan + localWatchSafetyInterval. For example, a targeted cycle at 59s followed by a local wake can keep the worker in debounce beyond the 60s deadline; with a configured scan interval above one minute the delay can be much longer. An unobserved local edit consequently misses the promised one-minute completeness backstop. Bound the inner debounce by the remaining safety deadline as well. This continues the existing safety-deadline thread; no duplicate inline is posted.

Verification

  • git diff --check origin/pr/329-base...HEAD: passed.
  • Go tests, race, vet and build: not run, 0 / 0 / 0 before discovery (go executable unavailable). No Darwin or Windows runtime check was performed.
  • The author reports full Go and race checks on this head; those are not independently executed here. The previously reported symlink deletion defect predates this PR and is tracked in #331.

@isac322
isac322 requested a review from runbear-bot November 3, 2026 03:35

@isac322 isac322 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GREEN on 547e1da. PR-ready: false while current-head CI and the requested runbear-bot re-review are pending.

The remaining safety-deadline finding is closed: both outer polling and inner watcher debounce honor the deadline, and an elapsed deadline cannot take the echo-only early-out. The same no-sleep regression fails on 4a36197 and passes on this source, including 10 race iterations. Full Go tests, vet, build, daemon/syncer race, and the exact webapp CI race command all passed locally after current main was merged. Previously verified remote receipt, bounded local discovery, queued local change ordering, and WebSocket fixture shutdown remain covered.

Windows-specific watcher behavior was not executed. The pre-existing symlink deletion defect remains tracked separately in #331. This author account records its verdict as a comment rather than a self-approval.

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: not-ready

  • 기존 safety deadline 스레드의 보완이 아직 불완전합니다. waitFor를 cycle 전에 남은 시간으로 계산한 뒤 cycle이 끝난 시점부터 다시 기다립니다. 예를 들어 마지막 full scan 10초 후 시작한 targeted cycle이 55초 걸리면 deadline은 60초인데, 이벤트가 없을 때 다음 검사 시점은 115초가 됩니다. cycle 이후 절대 deadline 기준으로 남은 시간을 다시 계산하고, 긴 cycle이 deadline을 넘는 회귀 테스트를 추가해 주세요. 기존 스레드와 같은 기전이므로 중복 inline은 추가하지 않았습니다.

검증: git diff --check 통과. 현재 소스의 clamp → cycle → wait 순서와 긴 cycle 시간 모델을 별도로 확인했습니다(실제 worker 실행 테스트는 아님). Go 실행 파일이 없어 daemon/syncer 테스트는 실행하지 못했습니다: 0 / 0 / 0. 작성자의 Go 테스트 결과는 작성자 보고이며 독립 실행 결과가 아닙니다.

@isac322
isac322 requested a review from runbear-bot November 3, 2026 04:34

@isac322 isac322 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GREEN on b2b462a. PR-ready: false until current-head CI and the requested bot re-review complete.

The safety wait now uses the remaining absolute deadline after the cycle, rather than adding a pre-cycle duration after long remote work. Inner debounce also honors that deadline. The new real-worker test fails on 547e1da after receiving peer content but missing local safety discovery, and passes after this fix. Long-cycle and debounce regressions passed 10 race iterations. Full Go tests, vet, build, daemon/syncer race and the exact webapp CI race command passed on the committed source. No new blocking finding was identified in the wait-path review.

Windows-specific watcher execution remains unverified; the pre-existing symlink deletion issue is tracked separately in #331. This author account records the internal verdict as a comment.

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: not-ready

현재 HEAD b2b462adaf2568173ca751d92675a64ca6de2b17 기준입니다.

  • P1 — read-only materialization이 symlink 경계를 우회합니다. 이번에 추가된 writable 경로의 Lstat/타입 검사는 read-only 경로에는 적용되지 않습니다. 캐시된 regular file을 로컬 symlink로 바꾼 뒤 peer 파일을 materialize하면 os.Stat와 keepLocalCopy가 symlink target을 따라가므로 mount 밖 파일을 hash/read하여 local copy로 복사할 수 있습니다. inline finding을 게시했습니다. read-only 경로도 Lstat 기반 타입 검사를 통과한 경우에만 regular-file 처리/복사를 수행해야 합니다.
  • P2 — cycle panic 복구가 targeted hint를 재시도할 수 있습니다. cyclePanicked 경로는 backoff 후 forceFull을 세우거나 nextHints를 비우지 않습니다. 건강한 watcher 상태에서 targeted cycle이 panic하면 같은 hint를 반복해 full discovery를 재설정하지 못할 수 있습니다. 이 finding은 기존 recovery branch의 상태 전이 문제라 body-only로 남겼습니다.

이전의 watcher rebuild/shutdown race, nested-root routing, remote wake의 local batch ordering, safety deadline/debounce/post-cycle timing finding은 현재 소스와 테스트 추가분에서 재현되지 않았습니다.

검증

  • git diff --check origin/pr/329-base...HEAD: passed.
  • Go 검증: go test ./internal/daemon ./internal/syncer, go test ./..., go vet ./..., go build ./... 모두 실행 전 차단 — 이 checkout에 go 실행 파일이 없어 각각 0 / 0 / 0 (blocked before discovery)입니다.
  • Darwin/Windows 실환경 검증: not run.
  • 독립 OMX draft는 현재 checkout/head에서 완료되었고 위 두 finding을 확인했습니다.

Comment thread internal/syncer/syncer.go
if ok && c.Blob == want.Blob && c.Mode == want.Mode && !s.readOnlyDrifted(rel, abs, c) {
return false, nil
}
if ok && !s.readOnlyPath(rel) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 새 guard는 !s.readOnlyPath(rel)일 때만 실행됩니다. 따라서 read-only prefix의 cached regular file이 로컬 symlink로 바뀌면 아래 os.Stat(abs)와 keepLocalCopy(abs)가 symlink target을 따라갑니다. target이 mount 밖이어도 그 내용을 hash/read하여 (local, not synced) 복사본으로 만들 수 있으므로, read-only 경로에도 Lstat 기반 타입 검사를 적용해 symlink를 regular-file materialization 경로에서 차단해야 합니다.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The referenced read-only handling predates this PR. I compared the original base eaa6e0d with the current branch and reproduced the same behavior on both using synthetic test data. The new writable-path guard does not change that read-only branch. I am leaving this separate concern out of the watcher-scoped diff and keeping further details out of this public thread.

@isac322

isac322 commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

The targeted-panic recovery finding is fixed in a2ab967. The panic branch sets forceFull and clears consumed hints before its existing cancellation/backoff select. A real-worker regression with a frozen safety clock and blocked watcher admission fails on b2b462a, then passes after the reset, including 10 race iterations. It verifies both the hinted edit and an otherwise unobserved file are uploaded after recovery. Full Go tests, vet, build, daemon/syncer race and the exact webapp CI race command pass locally. The separate read-only comment was evaluated against the original base and answered in its thread.

@isac322
isac322 requested a review from runbear-bot November 3, 2026 05:37

@runbear-bot runbear-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bobb's bot(Totoro) Review

Conclusion: ready-with-notes

현재 HEAD a2ab967176053432463ba3a84298d5a7ee90d767 기준 merge-blocking finding은 없습니다.

  • watcher 교체/종료 동기화와 same-volume nested mount 라우팅을 재확인했습니다.
  • remote wake/debounce 중 pending local batch 수집과 one-minute safety full-scan deadline을 재확인했습니다.
  • targeted materialization의 writable symlink/type 경계와 untracked symlink 보존을 재확인했습니다.
  • body-only review이며 changed-line inline comment는 게시하지 않습니다.

검증 및 한계

  • git diff --check origin/pr/329-base...HEAD: 통과.
  • Go toolchain이 없어 go test ./internal/daemon ./internal/syncer, go vet ./internal/daemon ./internal/syncer, go build ./..., gofmt는 실행 전 차단됨: 각 0 / 0 / 0 (blocked: go: command not found).
  • race detector와 Darwin/Windows 실환경 watcher 검증은 실행하지 않았습니다.
  • 작성자/기존 댓글의 테스트 결과는 독립 실행 결과로 집계하지 않았습니다.

@isac322
isac322 merged commit 844e708 into main Nov 3, 2026
6 checks passed
@isac322
isac322 deleted the isac322/daemon-rescans-the-whole-tree-on-every-watcher-w branch November 3, 2026 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Daemon rescans the whole tree on every watcher wake, including wakes from its own writes

2 participants