Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 52 additions & 8 deletions packages/cli/src/commands/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
removeHash,
renameHash,
computeHash,
shouldExcludeFromHash,
} from "../utils/template-hash.js";
import { compareVersions } from "../utils/compare-versions.js";
import { toPosix } from "../utils/posix.js";
Expand Down Expand Up @@ -1035,16 +1036,56 @@ function analyzeChanges(
return result;
}

function collectMissingManagedFileHashes(
/**
* Receipt entries that are wrong or missing for a file that is already
* byte-identical to its template.
*
* Nothing else repairs these. `analyzeChanges` classifies such a file
* `unchanged`, and the write-back draws only from `newFiles`,
* `autoUpdateFiles` and overwritten `changedFiles` — so a poisoned or absent
* entry beside a pristine file survives every subsequent `trellis update`,
* however many times it is run. Identical template content across versions is
* not what saves such an entry from going stale; it is precisely what freezes
* it, because the file never leaves the `unchanged` bucket.
*
* Recording the template's hash here cannot bless a local edit. Membership in
* `unchangedFiles` means the file on disk already *is* the template, byte for
* byte, so the value written is the one a correct receipt would already hold.
* A genuinely customized file differs from its template, lands in
* `changedFiles`, and is never seen by this function.
*
* That also leaves the mixed-ownership paths — `AGENTS.md`,
* `.github/copilot-instructions.md`, `.trellis/config.yaml` — free to differ
* from their recorded hash, which for them is the correct state: once the
* repository has appended its own content they are no longer `unchanged`.
*/
function collectUnchangedFileHashRepairs(
changes: ChangeAnalysis,
hashes: TemplateHashes,
): Map<string, string> {
const files = new Map<string, string>();
const managedFiles = new Set([FILE_NAMES.AGENTS, COPILOT_INSTRUCTIONS_PATH]);

for (const file of changes.unchangedFiles) {
if (managedFiles.has(file.relativePath) && !hashes[file.relativePath]) {
files.set(file.relativePath, file.newContent);
const key = toPosix(file.relativePath);
const recorded = hashes[key];

if (recorded === undefined) {
// A missing entry is only an omission for a path the receipt is meant
// to carry. `EXCLUDE_FROM_HASH` holds paths deliberately left out —
// `.trellis/.gitignore` among them — and adding those here would put
// this path in disagreement with `initializeHashes` about what the
// receipt tracks at all.
if (!shouldExcludeFromHash(key)) {
files.set(key, file.newContent);
}
continue;
}

// 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);
Comment on lines +1084 to +1088
}
}

Expand Down Expand Up @@ -2378,7 +2419,7 @@ export async function update(options: UpdateOptions): Promise<void> {

// Analyze changes (pass hashes for modification detection)
const changes = analyzeChanges(cwd, hashes, templates);
const missingManagedFileHashes = collectMissingManagedFileHashes(
const unchangedFileHashRepairs = collectUnchangedFileHashRepairs(
changes,
hashes,
);
Expand Down Expand Up @@ -2428,8 +2469,11 @@ export async function update(options: UpdateOptions): Promise<void> {
!hasPendingMigrations &&
!hasSafeDeletes
) {
if (!options.dryRun && missingManagedFileHashes.size > 0) {
updateHashes(cwd, missingManagedFileHashes);
// The "already up to date" exit still has to repair the receipt: this is
// exactly the clean tree where every file is `unchanged`, so it is the run
// where a wrong entry would otherwise be skipped again.
if (!options.dryRun && unchangedFileHashRepairs.size > 0) {
updateHashes(cwd, unchangedFileHashRepairs);
}

if (isSameVersion) {
Expand Down Expand Up @@ -2698,7 +2742,7 @@ export async function update(options: UpdateOptions): Promise<void> {
updateVersionFile(cwd);

// Update template hashes for new, auto-updated, and overwritten files
const filesToHash = new Map<string, string>(missingManagedFileHashes);
const filesToHash = new Map<string, string>(unchangedFileHashRepairs);
for (const file of changes.newFiles) {
filesToHash.set(file.relativePath, file.newContent);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/utils/template-hash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ const EXCLUDE_FROM_HASH = [
/**
* Check if a path should be excluded from hash tracking
*/
function shouldExcludeFromHash(relativePath: string): boolean {
export function shouldExcludeFromHash(relativePath: string): boolean {
const normalizedPath = toPosix(relativePath);
for (const pattern of EXCLUDE_FROM_HASH) {
if (normalizedPath.includes(pattern)) {
Expand Down
124 changes: 124 additions & 0 deletions packages/cli/test/commands/update.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1582,4 +1582,128 @@ describe("update() integration", () => {
computeHash(updated),
);
});

describe("receipt repair for files already identical to their template", () => {
/**
* Paths whose recorded hash is *expected* to drift: the repository appends
* its own content after the template is written, so the receipt holds the
* template's hash while the file on disk is longer. A fix that makes these
* "match" has broken them.
*/
const MIXED_OWNERSHIP = [
FILE_NAMES.AGENTS,
COPILOT_INSTRUCTIONS_PATH,
`${DIR_NAMES.WORKFLOW}/config.yaml`,
];

/**
* Every receipt entry whose recorded hash disagrees with the file at that
* path. Enumerated from the receipt itself rather than from a list of
* paths this test already knows about — a check built from what we expect
* cannot find the entry we did not expect.
*/
function mismatchedEntries(): string[] {
const hashes = readHashesV2(hashFilePath());
return Object.entries(hashes)
.filter(([relativePath, recorded]) => {
const full = projectFile(relativePath);
if (!fs.existsSync(full) || fs.statSync(full).isDirectory()) {
return false;
}
return computeHash(fs.readFileSync(full, "utf-8")) !== recorded;
})
.map(([relativePath]) => relativePath)
.filter((relativePath) => !MIXED_OWNERSHIP.includes(relativePath));
}

it("records a hash matching the file for every entry, right after init", async () => {
await setupProject();
expect(mismatchedEntries()).toEqual([]);
});

it("repairs a poisoned entry for a file that matches its template", async () => {
await setupProject();

// Poison one entry with a hash of different content, leaving the file
// itself pristine. This is the shape found in the fleet audit: the
// recorded value was another platform's template.
const hashes = readHashesV2(hashFilePath());
const victim = MANAGED_FILE;
const correct = hashes[victim];
expect(correct).toBeDefined();
hashes[victim] = computeHash("not what is on disk");
writeHashesV2(hashFilePath(), hashes);
expect(mismatchedEntries()).toContain(victim);

await update({ yes: true });

// One run is enough. Before this fix the file was classified `unchanged`
// on every run and skipped, so the entry could never be repaired.
expect(readHashesV2(hashFilePath())[victim]).toBe(correct);
expect(mismatchedEntries()).toEqual([]);
});

it("adds an entry that is missing entirely for a pristine file", async () => {
await setupProject();

const hashes = readHashesV2(hashFilePath());
const correct = hashes[MANAGED_FILE];
writeHashesV2(
hashFilePath(),
removeHashEntry(hashes, MANAGED_FILE) as Record<string, string>,
);
expect(readHashesV2(hashFilePath())[MANAGED_FILE]).toBeUndefined();

await update({ yes: true });

expect(readHashesV2(hashFilePath())[MANAGED_FILE]).toBe(correct);
});

it("does not re-hash a file the user actually customized", async () => {
await setupProject();

const pristine = readProjectFile(MANAGED_FILE);
const recorded = readHashesV2(hashFilePath())[MANAGED_FILE];
expect(recorded).toBe(computeHash(pristine));

// A real local edit. `update` may offer to overwrite it; declining must
// leave the receipt describing the template, not the edit — otherwise
// the repair path has silently blessed a customization.
const customized = `${pristine}\n# local customization\n`;
writeProjectFile(MANAGED_FILE, customized);
vi.mocked(inquirer.prompt).mockResolvedValue({ proceed: false });

await update({ yes: false });

expect(readHashesV2(hashFilePath())[MANAGED_FILE]).not.toBe(
computeHash(customized),
);
expect(readProjectFile(MANAGED_FILE)).toBe(customized);
});

it("leaves the mixed-ownership paths free to differ from their recorded hash", async () => {
await setupProject();
const before = readHashesV2(hashFilePath());

// Append repo-owned content, exactly as those files acquire it.
for (const relativePath of MIXED_OWNERSHIP) {
if (!fs.existsSync(projectFile(relativePath))) continue;
writeProjectFile(
relativePath,
`${readProjectFile(relativePath)}\n# repo-owned addition\n`,
);
}

await update({ yes: true });

const after = readHashesV2(hashFilePath());
for (const relativePath of MIXED_OWNERSHIP) {
if (before[relativePath] === undefined) continue;
// The recorded hash must still describe the template, not the file.
expect(after[relativePath]).not.toBe(
computeHash(readProjectFile(relativePath)),
);
}
});
});
});
Loading