Skip to content

Leave verifier refs out of a push for a skill you do not own - #60

Merged
rhiever merged 1 commit into
mainfrom
fix/shared-skill-verifier-noop
Jul 20, 2026
Merged

Leave verifier refs out of a push for a skill you do not own#60
rhiever merged 1 commit into
mainfrom
fix/shared-skill-verifier-noop

Conversation

@rhiever

@rhiever rhiever commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

The bug

goodeye skills sync push could not update a skill shared with you, even at edit access. The push was reported as a read-only skip, which made it look like the grant was wrong.

The payload was the problem. Only a skill's owner may change its verifier references, but the push sent them on every save regardless of who owned the skill. _verifier_payload returned a list rather than None, and the client only drops a key whose value is None, so even a skill with no bindings went out carrying "verifiers": []. A registry that treats the field's presence as an attempt to set it then refuses the entire save.

The fix

Return None for a skill someone else owns, which leaves the key out of the payload. The registry carries the stored references forward whenever the field is absent, so nothing is lost by omitting it.

Bindings the registry reports back are still recorded in the local index, so it keeps tracking references the caller cannot edit.

An index written by an older version has no recorded role and loads as owner, so it keeps sending the field as before. That path is unaffected by this change and is handled on the registry side.

Testing

  • New test asserts the key is absent for an edit-granted entry, that the save still addresses the skill by id, and that returned bindings are still recorded locally. Written red first.
  • Full suite passes (1043).

There is a companion change on the registry side so that already-installed versions keep working without waiting on a release.

🤖 Generated with Claude Code

Only a skill's owner may change its verifier references, so a push for a
skill shared with you has nothing to say about them. The registry carries
the stored references forward whenever the field is absent, so omitting it
is both correct and sufficient.

The push payload sent them regardless. `_verifier_payload` returned a list
rather than `None`, and the client only drops a key whose value is `None`,
so a skill with no bindings still went out with an empty list attached. A
registry that reads the field's presence as an attempt to set it refuses
the whole save, which surfaced here as a read-only skip and made an
ordinary edit look like a missing grant.

Return `None` for a skill someone else owns so the key is left out.
Bindings the registry reports back are still recorded in the index, so it
keeps tracking references the caller cannot edit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rhiever
rhiever merged commit ac7da82 into main Jul 20, 2026
1 check passed
@rhiever
rhiever deleted the fix/shared-skill-verifier-noop branch July 20, 2026 19:42
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.

1 participant