Skip to content

fix(update): prevent collisions in project skill hashes - #2438

Open
Osraka wants to merge 1 commit into
vercel-labs:mainfrom
Osraka:fix/project-skill-hash-framing
Open

Osraka wants to merge 1 commit into
vercel-labs:mainfrom
Osraka:fix/project-skill-hash-framing

Conversation

@Osraka

@Osraka Osraka commented Oct 8, 2026

Copy link
Copy Markdown

Summary

Fixes #2427. computeSkillFolderHash currently concatenates each filename and its bytes without a boundary. A skill with file a containing bc therefore hashes identically to one with file ab containing c, even when the rest of the skill is unchanged. Project skills update can treat the changed skill as current.

Frame each file as its relative path, a NUL separator, its byte length, another NUL separator, then its contents. Filesystem paths cannot contain NUL, and the byte length makes content boundaries unambiguous. No lockfile schema or install behavior changes.

Verification

  • Added a filesystem regression with identical SKILL.md files and the a/bc versus ab/c pair. It failed on current main with equal hashes and passes after the change.
  • vitest run: 941 passed (69 files), using an isolated HOME.
  • pnpm type-check, pnpm format:check, and pnpm build passed.

Compatibility

Existing 64-character project computedHash values remain readable but will differ under the new framing. The next project update may reinstall a skill once and then record the new hash. GitHub tree SHA tracking in the global lock is unchanged.

@vercel-agent-factory vercel-agent-factory Bot added bug Something isn't working triage:bug-fix Factory triage found an evidence-backed bug-fix candidate for maintainer review labels Oct 8, 2026
@vercel-agent-factory

Copy link
Copy Markdown

@quuu — bug-fix candidate.

  • Fix: project skill hash could collide when a file's name and content length shifted (e.g. a/bc vs ab/c), letting skills update treat a changed skill as unchanged.
  • Tests: added regression in tests/local-lock.test.ts asserting the two layouts now hash differently.
  • CI: pending/approval-gated (head 8c8cdfcb).

@antfu antfu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The only concern is that this would change all the hashes for existing locks, but I think this is a necessary fix.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working triage:bug-fix Factory triage found an evidence-backed bug-fix candidate for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Folder hash misses a filename and content change

2 participants