fsmonitor: fix late cookies and preserve index history - #66
Draft
ttaylorr-oai wants to merge 6 commits into
Draft
Conversation
ttaylorr-oai
force-pushed
the
ttaylorr/codex-unstable-feedback-fixes
branch
from
August 21, 2026 18:54
d17c9bf to
3b52212
Compare
ttaylorr-oai
changed the title
ttaylorr-oai
force-pushed
the
ttaylorr/codex-unstable-feedback-fixes
branch
from
August 21, 2026 20:02
490f4d4 to
678fb2d
Compare
ttaylorr-oai
force-pushed
the
codex-unstable
branch
from
August 24, 2026 18:44
f906482 to
7e54125
Compare
with_lock__wait_for_cookie() gives a filesystem provider one second to report a synchronization cookie. A healthy FSEvents stream can miss that deadline while macOS is under load. The daemon then returns a trivial response, and status scans the entire index even though event delivery is still making progress. 4b1c56a (fsmonitor: flush pending FSEvents before cookie wait, 2026-07-21) requested an asynchronous flush on every Darwin query but kept the same one-second deadline. f439708 (Revert "fsmonitor: flush pending FSEvents before cookie wait", 2026-08-17) reverted it after a matched 48-query test still saw 12 timeouts in each arm. Avoid restoring that unqualified hot-path request. When the initial Darwin wait expires, request an asynchronous FSEvents flush and wait one more bounded interval. Successful queries retain the original wait and do not issue a flush or extend their deadline. The asynchronous call cannot block on the callback while the client holds main_lock. If the provider stays silent, retain the existing trivial-response fallback after the retry. Add a test-only callback delay to exercise both outcomes: a 1.2-second delay is recovered, while a 2.5-second delay still reaches the bounded fallback.
The daemon currently assumes that each client which advances an FSMonitor token also updates the repository's canonical index. That does not hold for commands using GIT_INDEX_FILE. A private index can advance the daemon past the canonical index's token and cause the canonical index's next query to receive a global invalidation. Keep a deduplicated overflow batch instead of discarding old paths. Clients at the overflow sequence still get an exact delta. Older clients get a conservative union of paths, which may overreport but cannot miss a change. All paths are interned. Keep a pointer-identity hash set with the overflow batch so later compactions hash only newly retired paths, rather than rebuilding a set over the daemon's lifetime history. Add a regression which advances a private index repeatedly, verifies that compaction remains deduplicated, and then checks that a read-only canonical status reports both changed files without a trivial response.
Retired batches are collapsed into a path-only overflow set. That keeps old indexes complete, but it loses the sequence in which each path was last observed. A client that consumed an inode event can therefore see it again after another index compacts the batch list, causing repeated hard-link scans. Unpinned batches have a zero pinned time and are also eligible for compaction immediately despite the default grace period. Do not use unpinned batches as truncation boundaries. Record the newest original batch sequence for every overflow path, and filter overflow responses against the client's requested sequence. The normal batch walk remains unchanged; sequence lookups are confined to overflow responses. Cover both the default retention grace and the cross-index hard-link case. The latter persists a nonzero checkpoint, compacts through a private index, and verifies repeated canonical reads do not rescan or fall back to global invalidation.
The delayed-cookie tests send the v1 timestamp token "0" and only check that the response is nonempty. Both recovery and fallback can satisfy that assertion with the same trivial response, so the tests do not distinguish a rescued cookie from a token-generation reset. Send a deterministic valid v2 token instead. Verify that the 1200ms case preserves its token generation without a global invalidation, while the 2500ms case changes generation and sends the fallback invalidation.
ttaylorr-oai
force-pushed
the
ttaylorr/codex-unstable-feedback-fixes
branch
from
August 24, 2026 19:29
a646dbf to
2f97399
Compare
215845a (fsmonitor: preserve authenticated proofs across ordinary commands, 2026-08-15) enabled the clean-status history handoff for merges, but excluded invocations where fast_forward was FF_NO. Requested merge topology does not determine whether the resulting index is semantically safe. A clean non-fast-forward merge can carry the same authenticated FSUC/FSCF state as a fast-forward merge. As a result, --no-ff, --no-ff --no-commit, and merge.ff=false all dropped FSUC and reduced the FSCF flags from 15 to 9 after a clean merge. Each subsequent read-only status invalidated the external history and rescanned the semantic manifest. Enable the handoff for every merge using the canonical index. Conflict handling still invalidates unsafe proofs, and explicit alternate indexes remain excluded. Cover all three non-fast-forward forms, repeated read-only status calls, conflicts, and alternate indexes.
ttaylorr-oai
added
kind:review-only
An exact clean status can repair a stale FSMonitor checkpoint or cached stat data while it scans. The repair requires an index write, so the existing issue path leaves no clean sidecar behind. Read-only callers then repeat the full scan until a second writable exact status publishes the proof. After the repair is written and resumable history is durable, install a sidecar bound to the rewritten index. Keep optional-lock-disabled commands read-only, preserve the literal exact-command restriction, and do not extend sidecar support to linked worktrees. Cover repeated read-only scans after a legacy daemon replacement, the single writable index repair in main and linked worktrees, and the next read-only sidecar hit in the main worktree. Keep option-bearing status commands ineligible for proof publication.
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
GIT_INDEX_FILEcannot prune history still needed by the canonical indexValidation
make -j8./t7527-builtin-fsmonitor.sh(175/175)make -C t test-lintgit diff --checkgit statusscenarios over 12,000 tracked paths; every trace reported zero compactions. The wall-clock data was nonstationary, so it is retained as inconclusive rather than used as performance evidence.