Skip to content

fix(sync-state): drop per-skill content hash, use git ref for identity#389

Merged
christso merged 1 commit into
mainfrom
fix/388-revert-content-hash
May 12, 2026
Merged

fix(sync-state): drop per-skill content hash, use git ref for identity#389
christso merged 1 commit into
mainfrom
fix/388-revert-content-hash

Conversation

@christso
Copy link
Copy Markdown
Contributor

Summary

Reverts the per-skill content-hash machinery from #382. Identity for git-based plugins is already provided by url + refgit rev-parse HEAD after each fetch returns the commit SHA, which resolvedSha already records on the source entry. Hashing every installed skill folder on every install was CPU work for no win over what resolvedSha gives for free.

Mapping the issue's naming to this codebase:

  • "lock file" = .allagents/sync-state.json's sources block
  • "LockFileEntry" = SyncStateSourceSchema
  • "contentHash" = SyncStateSkillSchema.contentHash (per-skill, removed)
  • "ref" = resolvedRef + resolvedSha
  • "lastSynced" = top-level lastSync (already there)

Changes

  • src/models/sync-state.ts — remove SyncStateSkillSchema and the skills map on SyncStateSourceSchema. Each lock entry is now just { pluginSpec, resolvedRef, resolvedSha, pinnedRef? }.
  • src/core/sync-state.ts — drop computeSkillFolderHash, upsertSyncStateSkill, and the unused crypto.createHash / readdir / relative imports. The private persistMergedSyncState helper folds back into upsertSyncStateSource.
  • src/cli/commands/plugin-skills.ts — rename recordContentProvenancerecordSourceProvenance and drop the now-unused skills param + skill-folder walk. Both callers (--from <skill> install and --all) updated.

There was no needsSync hash-comparison logic in the codebase — the only consumer of the hash was the skill update command, which was removed in #387.

Test plan

  • bun run build
  • bun run typecheck
  • bun test — 1209 pass / 0 fail
  • End-to-end: skill add brainstorming --from obra/superpowers --pin v3.1.0 writes
    {
      "obra/superpowers": {
        "pluginSpec": "obra/superpowers",
        "resolvedRef": "v3.1.0",
        "resolvedSha": "0bc5a5989d8cf140e9f671af14c76f6a6e6fae79",
        "pinnedRef": "v3.1.0"
      }
    }
    No skills map.
  • No remaining references to computeSkillFolderHash, upsertSyncStateSkill, SyncStateSkill, contentHash, or skillFolderHash anywhere in src/ or tests/.

Closes #388

Reverts the per-skill content-hash machinery added in #382. For git-based
plugins, identity is already given for free by `url + ref` — `git rev-parse
HEAD` after each fetch returns the commit SHA, which is what `resolvedSha`
already records on the source entry. Computing a per-file sha256 tree over
every installed skill folder on every install adds CPU work and code
complexity without giving us anything `resolvedSha` doesn't.

- src/models/sync-state.ts: remove `SyncStateSkillSchema` and the `skills`
  map on `SyncStateSourceSchema`. Each lock entry is now just
  `{ pluginSpec, resolvedRef, resolvedSha, pinnedRef? }`. The top-level
  `lastSync` already records the timestamp.
- src/core/sync-state.ts: drop `computeSkillFolderHash`, `upsertSyncStateSkill`,
  and the unused `crypto.createHash` / `readdir` / `relative` imports. The
  private `persistMergedSyncState` helper folds back into
  `upsertSyncStateSource`.
- src/cli/commands/plugin-skills.ts: rename `recordContentProvenance` →
  `recordSourceProvenance` and drop the now-unused `skills` param + skill
  folder walk. The two callers (`--from <skill>` install and `--all`)
  no longer need to thread `skills` through.

Verified end-to-end: `skill add brainstorming --from obra/superpowers --pin
v3.1.0` writes a `sources["obra/superpowers"]` entry with `pluginSpec`,
`resolvedRef`, `resolvedSha`, `pinnedRef` only — no `skills` map.

`bun test`: 1209 pass / 0 fail.

Closes #388
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying allagents with  Cloudflare Pages  Cloudflare Pages

Latest commit: b91619d
Status: ✅  Deploy successful!
Preview URL: https://0fbc8007.allagents.pages.dev
Branch Preview URL: https://fix-388-revert-content-hash.allagents.pages.dev

View logs

@christso christso merged commit c9fb7ee into main May 12, 2026
1 check passed
@christso christso deleted the fix/388-revert-content-hash branch May 12, 2026 21:45
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.

Revert content hash identity — use git ref instead

1 participant