Skip to content

fix(update): repair receipt entries for files already identical to a template - #575

Open
sdelmas wants to merge 1 commit into
mindfold-ai:mainfrom
sdelmas:upstream-pr/update-receipt-repair
Open

fix(update): repair receipt entries for files already identical to a template#575
sdelmas wants to merge 1 commit into
mindfold-ai:mainfrom
sdelmas:upstream-pr/update-receipt-repair

Conversation

@sdelmas

@sdelmas sdelmas commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Slice B of the #534 resplit. Branched off current main (64e66369). Independent of #574 — different files, no ordering constraint between them.

The bug

.template-hashes.json recorded hashes that disagreed with the files written, and omitted entries for files that exist. Either way the receipt stops working as a drift signal: a clean vendored tree reports as locally modified, so genuine customizations can't be told apart from noise. This surfaced during an 8-repo rollout where the receipt was the tool for deciding which local edits were real — it produced false positives on every repo.

Root cause

analyzeChanges classifies a file whose content already equals its template as unchanged. The write-back drew only from newFiles, autoUpdateFiles, and overwritten changedFilesunchangedFiles was never written back. So a wrong or absent entry sitting beside an already-correct file could not be repaired by any number of trellis update runs.

Worth stating plainly because it inverts the original reasoning: identical content across versions is not what keeps such an entry fresh — it is exactly what freezes it, because the file never leaves the unchanged bucket.

An earlier reading blamed key construction for dropping a platform segment. That was disproved: the collector emits the correct hash, and 0.6.7 hashed bytes read from disk. The keys were always right; the repair path was always missing.

Files (3)

  • cli/src/commands/update.ts — write back unchangedFiles too
  • cli/src/utils/template-hash.ts
  • cli/test/commands/update.integration.test.ts — +124 lines

The source commit also carried .trellis/tasks/08-19-template-hashes-stale/; dropped per your scrub list.

Testing

pnpm build   → ok
cli          → Test Files 76 passed | Tests 1713 passed   (+5 receipt tests)
core         → Test Files 19 passed | Tests 346 passed, 1 skipped

No .trellis/ paths, no -sd.N identity (stays 0.6.15), no marketplace gitlink.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved update repair behavior for unchanged template files with missing or incorrect hash records.
    • Hash records are now repaired during both regular updates and already-up-to-date checks.
    • Files intentionally excluded from hash tracking remain unaffected.
    • Customized files and mixed-ownership files are no longer incorrectly re-hashed.

…template

`.template-hashes.json` recorded hashes that disagreed with the files
written, and omitted entries for files that exist. Both make the receipt
useless as a drift signal: a clean vendored tree reports as locally
modified, so real customizations cannot be told apart from noise.

Root cause: `analyzeChanges` classifies a file whose content already
equals its template as `unchanged`, and the write-back drew only from
`newFiles`, `autoUpdateFiles` and overwritten `changedFiles`.
`unchangedFiles` was never written back, so a wrong or absent entry
beside an already-correct file could not be repaired by any number of
`trellis update` runs.

Identical content across versions is not what saves such an entry from
going stale — it is exactly what freezes it, because the file never
leaves the `unchanged` bucket.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 25, 2026 15:23
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 96c05aef-e919-4ba7-95dc-87be986c5534

📥 Commits

Reviewing files that changed from the base of the PR and between 64e6636 and 0344d52.

📒 Files selected for processing (3)
  • packages/cli/src/commands/update.ts
  • packages/cli/src/utils/template-hash.ts
  • packages/cli/test/commands/update.integration.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The update command now repairs missing or incorrect hash receipts for unchanged template files. It preserves exclusions and avoids re-hashing customized or mixed-ownership paths. Integration tests cover repair behavior in normal and already-up-to-date update flows.

Changes

Template hash receipt repair

Layer / File(s) Summary
Unchanged-file repair selection
packages/cli/src/commands/update.ts, packages/cli/src/utils/template-hash.ts
Exports shouldExcludeFromHash and uses it while collecting repairs for missing or mismatched hashes on unchanged template files.
Repair persistence and integration coverage
packages/cli/src/commands/update.ts, packages/cli/test/commands/update.integration.test.ts
Writes repairs during normal and already-up-to-date updates. Tests cover pristine files, missing entries, customized files, and mixed-ownership paths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 0344d

The change repairs receipt entries for files already matching their templates and includes passing build and test results; no actionable merge-blocking risk remains.

Suggested reviewers: cnhlaia, taosu0216

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: repairing receipt entries for files that are already identical to their templates.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes .template-hashes.json “receipt” staleness for files whose on-disk content is already byte-identical to their template, so incorrect/missing hash entries can be repaired even when analyzeChanges classifies those files as unchanged.

Changes:

  • Repair and write back missing/incorrect receipt entries for unchangedFiles during trellis update (including the “Already up to date” early-exit path).
  • Export shouldExcludeFromHash so update logic can avoid creating entries for intentionally untracked paths.
  • Add integration coverage for poisoned/missing entries, customized files, and mixed-ownership paths.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/cli/src/commands/update.ts Adds unchanged-file receipt repair and ensures the early “up to date” exit still updates the receipt.
packages/cli/src/utils/template-hash.ts Exposes shouldExcludeFromHash for reuse by update-time repair logic.
packages/cli/test/commands/update.integration.test.ts Adds integration tests to validate repair behavior and guard against re-hashing real user customizations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1084 to +1088
// An entry that already exists and disagrees with the file is repaired
// whatever the path: a wrong value is strictly worse than an absent one,
// because it reads as a real local modification.
if (recorded !== computeHash(file.newContent)) {
files.set(key, file.newContent);
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.

2 participants