status: preserve clean-status proofs across mixed Git writers - #21
Conversation
8358fcb to
949c04b
Compare
be8e9d4 to
2b48ff1
Compare
abc124f to
e106175
Compare
b910009 to
597f818
Compare
c06165f to
c7c8cd2
Compare
c7c8cd2 to
04327fa
Compare
04327fa to
f81994a
Compare
2b48ff1 to
7c7960a
Compare
6c58b4d to
52558de
Compare
adb86ea to
c9ef024
Compare
dreynaud-oai
left a comment
There was a problem hiding this comment.
Do not ship this head. Independent exact-tree macOS and Linux checks reproduce two false-clean cases: a same-second external hardlink edit after missing fsmonitor history, and a retained FSMN/FSUC edit that hides both a tracked .gitignore and a newly unignored file. The 1,038-commit attribute-history classifier does not run, invalid fsmonitor bits block clean-proof issuance, and eight Linux jobs fail, including legacy daemon token compatibility. I will re-review an exact successor only after dual-hash correctness, mixed-writer and daemon-upgrade gates, real-checkout latency, and exact-head CI pass.
c7854b1 to
0c521d4
Compare
An otherwise clean worktree containing untracked files cannot use the whole-worktree clean sidecar. Status still has a closed fsmonitor token and individually certified tracked entries, but previously reused that state only for unqualified top-level status. Allow the existing tracked-clean and cache-tree shortcuts for safely supported query shapes, including pathspecs, machine-readable formats, branch and stash headers, and nested working directories. Preserve the provider, semantic, token, expanded-index, and tracked-entry checks. Staging, unstaging, or switching branches also invalidated the semantic proof whenever the logical index changed. Preserve it when the affected entries cannot change attribute or ignore rules. Verify new directory ancestors with anchored, no-follow attribute probes, and reject filters, sparse indexes, resolve-undo state, and changed semantic sources. For ordinary branch switches, transfer semantic history only after each changed entry passes those same checks. Keep modified worktree entries fsmonitor-invalid and clean sidecars bound to the new logical index. Cover scoped queries, staging, unstaging, newly indexed directories, branch switches, and attribute changes with provider regressions.
Checking out an unchanged path currently rewrites the index even when no entries changed. This also affects "git restore", which uses the same path checkout machinery. Use SKIP_IF_UNCHANGED to avoid the write when the index is unchanged and no post-index-change hook is installed. Preserve the existing write when such a hook exists, since checkout has invoked it even for unchanged paths and t7113 explicitly covers that behavior. Actual worktree writes still refresh cached stat information and mark the index dirty, so those updates continue to be written. Mark sparse-directory entries dirty when replacing their object IDs in non-overlay mode. These in-place updates previously relied on the unconditional write and must not be skipped. On a repository with 1,000,001 tracked paths and a 103 MiB index, checking out an unchanged path improves from 242 ms to 32 ms. Add checkout and restore coverage while disabling fsmonitor within timestamp-sensitive tests, since fsmonitor metadata can itself dirty the index. Signed-off-by: Ted Nyman <tnyman@openai.com>
The exclude-race helper observes a Trace2 fallback marker before stopping its background status process. A fast fallback can exit successfully between that observation and kill, which made an otherwise correct race test fail nondeterministically. Keep terminating a process that is still running. If it has already exited, require wait to report successful completion instead of treating the failed signal as a test failure.
A commit dry run refreshes the index to report whether anything could be committed. Its as-is preparation also takes the real index lock and persists refreshed stat information, even when optional locks have been explicitly disabled. Avoid taking or writing the real index lock for an as-is dry run under --no-optional-locks. Keep the in-memory refresh and cache-tree update, so clean stat mismatches, genuine worktree changes, and staged changes produce the same result as before. Real commits and partial dry runs retain their existing locking behavior. Cover clean and dirty dry runs, a preexisting index lock, and the ordinary dry run that still persists its stat repair.
A mixed reset always writes the index after reading its target tree, even when its selected paths and stat information are already current. Replacing an identical index invalidates its physical clean-status proof and makes the following status rescan the repository. Skip the write only for a mixed reset with no index changes and no post-index-change hook. Continue taking the index lock, refreshing as requested, updating HEAD and ORIG_HEAD, and running configured hooks; hard, merge, and keep resets retain their existing behavior. Cover same-HEAD and pathspec resets, resetting to another commit with an identical tree, preserved ORIG_HEAD, and an installed hook that still forces the original index write.
Enabling status.showStash prevented every clean-status proof issuance path. Exact porcelain output is no longer empty when a stash exists, and ordinary root status rejected the stash decoration even after a complete verified worktree scan. Allow only the existing normal, root-wide issuance path to include the live stash summary. Preserve the exact-porcelain issuance restriction and every untracked, ignored, sparse, pathspec, and provider safety check. Exercise a real stash and configuration-namespace transition: exact porcelain saves reusable history without issuing a proof, ordinary status restores that history and issues one, and later long and porcelain queries reuse it while displaying current stash information.
Avoiding an index rewrite matters because the clean-status sidecar is bound to the physical index. A logically harmless checkout, restore, or mixed reset must leave both artifacts unchanged so the next status can reuse its existing proof. Exercise unchanged checkout and restore paths plus mixed HEAD and pathspec resets with the real fsmonitor provider. Require identical index and sidecar bytes, no index write, and an output-equivalent subsequent status without an index read, refresh, preload, or directory traversal.
With -u, checkout-index always commits its index lock after checking out the requested paths. An already-current entry leaves cache_changed clear, but the unconditional write still replaces a byte-identical index and invalidates a clean-status sidecar tied to its identity. Skip that write only when the index is unchanged and no post-index-change hook is installed. Keep taking the lock, writing genuine stat updates, and invoking configured hooks as before. Exercise the unchanged index and hook cases directly. Also cover path, force, all-files, and stdin invocations with a real fsmonitor daemon, requiring both the index and sidecar to survive and the next status to reuse its clean proof without reading the index.
A Git implementation without clean-status extensions can rewrite the same logical index while dropping its fsmonitor state. A later diff currently initializes an unusable timestamp token, receives a full invalidation from the daemon, scans every attribute directory, and stats every tracked entry even when a valid external checkpoint exists. Opt into existing external-history restoration only when a validated clean-status sidecar names the current configuration. Reject alternate indexes, non-main or sparse worktrees, configured clean filters, unsupported filesystems, and repositories without builtin fsmonitor. The existing checkpoint checks still validate the index, attributes, provider token, and staged entries before restoring any history. Reproduce a foreign rewrite without depending on another Git binary by rebuilding its index with fsmonitor disabled. Require a clean diff to restore the checkpoint without scanning worktree metadata, statting tracked entries, or rewriting the index, and verify that a subsequent real tracked change still appears in the diff.
Sparse-checkout reapply and repeated set or add commands always commit the index after updating sparsity, even when no entries or stat data changed. Replacing an identical index discards its physical identity and needlessly refreshes repository metadata. Skip the write only when the index and its pending worktree flags are unchanged and no post-index-change hook is installed. Explicit --sparse-index and --no-sparse-index requests set updated_workdir before converting the index, so retain their required rewrite even when cache_changed is clear. Preserve index locking, warnings, cleanup, and configured hook invocations. Cover all three settled no-op commands, the configured hook, and both explicit sparse-index format transitions.
0c521d4 to
dccdebb
Compare
Preserve the independently validated Git status preview tree while making its existing fourteen-patch integration directly reviewable against the current codex-unstable branch.
dccdebb to
cfb9444
Compare
dreynaud-oai
left a comment
There was a problem hiding this comment.
Independently verified exact reviewed tree a1034247a615ee89df8fb67c5e1f6c94a6855d99: mergeable .658-based provenance with no protected-workflow changes; 46 release stages / 836 checks; installed .658 and .644 mixed-writer matrices 85/85 in both object formats; real 1.16M-entry checkout stage 263 ms, unstage 239 ms, status after unstage 620 ms; macOS SHA-1/SHA-256 suites; Linux failure-injection and sanitizer suites; 92 passing exact-head CI checks. Live index retains zero zero-stat records and a fully valid untracked cache.
The APFS bulk-preload test barrier published its READY file before it
opened the resume FIFO. If the child was descheduled at that point, the
parent could observe READY, write and close the FIFO, and lose the byte
before any reader existed. The child then blocked forever while macOS CI
appeared hung.
Open the resume FIFO before publishing READY. The parent already holds
the FIFO open read/write before it starts status, so the child open does
not block; READY now proves that a reader owns the resume descriptor.
The previous topic tip is already published as
v2.55.0-openai.619.gb264c6f26648, so this correction is additive on topof that released history rather than rewriting it. Its resulting tree is
byte-identical to the separately audited owner-amended replay.
Validation:
preload_index_bulk_darwinunit suite (6/6)122/122 ordered parents