fix(devutils): keep string files' mtimes when the dev name pass round-trips them

Merged
NO
6h ago
+225-3

Problem. worktree.sh sync reverts rac's dev names (dev_names.py unsub) before quilt pops and pushes, then applies them again (sub). That rewrites about 2,700 .grd, .grdp and .xtb files. Their content is byte-identical, but their mtimes are new, so siso re-runs grit everywhere. On the 156 base, every stream that cherry-picked its commits and ran sync saw its next build plan 30,000 to 90,000 steps (UI's tools-156 showed 87,755), and the queue's cap stopped it.

Change. unsub records each substituted file's mtime and sha256 in build/src/.rac_names/mtimes.json. After substituting, sub puts the old mtime back on every file whose digest matches. Files a patch really changed keep their new mtime. The record also stores a build marker (the mtimes of out/*/.siso_fs_state and .ninja_log). If the tree was built between unsub and sub, for example after a failed sync, the record is dropped, so an old mtime is never put back on content the build has moved past. A corrupt or old-format record is dropped as well.

Verified.

  • test_dev_names.py: 23 tests, including a round-trip that keeps mtimes, a file changed by a patch keeping its new mtime, no record leaving behaviour unchanged, and a build in between dropping the record.
  • test_worktree.py: 39 tests.
  • Devin SWE-2 wrote it. Claude Sonnet reviewed it and closed the stale-record case. Claude Opus led.

Not verified. It hasn't been tried on a real 156 clone's sync yet. The first stream that cherry-picks it will see whether its post-sync build drops to the size of the patches it changed.

Patch surface (D17): no change, because this PR touches no patches.

šŸ¤– Generated with Claude Code

Devin Review

[!NOTE]
Medium Risk
Touches dev sync and build incremental correctness via filesystem mtimes; logic is guarded by hashes and build markers but has not been validated on a full 156 clone sync yet.

Overview
Fixes worktree.sh sync inflating the next siso build by re-touching ~2,700 string files whose content is unchanged after the dev name pass round-trip.

dev_names.py unsub now writes build/src/.rac_names/mtimes.json with each affected file's mtime_ns, sha256, and a build marker (mtimes of out/*/.siso_fs_state, .ninja_log, build.ninja, etc.). sub runs restore_mtimes afterward and resets the saved mtime only when the file digest still matches; patch-changed files keep their new mtime. If the marker differs (build or gn gen between unsub and sub), the record is discarded with a stderr warning; corrupt JSON is rejected the same way. A no-op unsub after a failed sync leaves the pending record so a later sub can still restore mtimes.

Adds run_passes and seven unit tests for round-trip mtimes, content changes, build/gn invalidation, malformed records, and retry behavior. docs/internals/overview.md describes the slimmer post-sync grit work and when mtimes are dropped.

Reviewed by Cursor Bugbot for commit 807018e4151f0751e18de8a9d0a054c1b5a5acd7. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by cubic

Keeps the mtimes of dev_names string files so worktree.sh sync no longer makes siso re-run grit on ~2,700 unchanged .grd, .grdp, and .xtb files. Previously the unsub/sub round-trip rewrote them with identical content but new mtimes, inflating the next build plan by 30,000ā€“90,000 steps.

Bug Fixes

  • unsub stores each file's mtime and sha256 in build/src/.rac_names/mtimes.json; sub restores the old mtime only for files the passes rewrote byte-identical, so patch-changed files keep their new mtime.
  • A build marker saved with the record drops it if any build ran between unsub and sub; it now also covers build.ninja and build.ninja.stamp, since a gn gen reads .grd files.
  • A malformed record is dropped with a warning instead of crashing, and a retried unsub after a failed sync keeps the pending record for the later sub.
  • Adds 7 tests covering mtime restoration, patch-changed files, retries, and record-dropping cases; docs/internals/overview.md documents the new behavior.
  • Not yet exercised against a real sync on the 156 base; the first stream to cherry-pick it will confirm the post-sync build drop.