fix invalid union types and add automated checks to CI #132
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| --- | ||
| "@hypercerts-org/lexicon": minor | ||
| --- | ||
|
|
||
| Convert app.certified.defs#did to object type | ||
|
|
||
| The did definition in app.certified.defs has been converted from a primitive | ||
| string type to an object type to comply with the ATProto specification | ||
| requirement that all union variants must be object or record types. | ||
|
|
||
| This change was necessary because app.certified.badge.award uses this | ||
| definition in a union for the subject property. | ||
|
|
||
| Breaking changes: | ||
|
|
||
| - `app.certified.defs#did`: Now an object with `did` string property (maxLength 256) | ||
| - Code using this type must now access the `.did` property instead of using the value directly | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| --- | ||
| "@hypercerts-org/lexicon": minor | ||
| --- | ||
|
|
||
| Convert union string definitions to object types in activity lexicon | ||
|
|
||
| The contributorIdentity, contributorRole, and workScopeString definitions | ||
| in org.hypercerts.claim.activity have been converted from primitive string | ||
| types to object types to comply with the ATProto specification requirement | ||
| that all union variants must be object or record types. | ||
|
|
||
| Additionally, maximum length constraints have been reduced to more reasonable | ||
| values: | ||
|
|
||
| - `contributorIdentity.identity`: maxLength 1000, maxGraphemes 100 (previously no limits) | ||
| - `contributorRole.role`: maxLength 1000, maxGraphemes 100 (previously maxLength 10000, maxGraphemes 1000) | ||
| - `workScopeString.scope`: maxLength 1000, maxGraphemes 100 (previously maxLength 10000, maxGraphemes 1000) | ||
|
|
||
| Breaking changes: | ||
|
|
||
| - `contributorIdentity`: Now an object with `identity` string property | ||
| - `contributorRole`: Now an object with `role` string property | ||
| - `workScopeString`: Now an object with `scope` string property | ||
| - Reduced maximum lengths may affect existing records with longer values | ||
|
|
||
| This requires updating code that uses these union types to access the nested | ||
| property instead of using the value directly. | ||
|
aspiers marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,17 @@ | |
| "description": "Common type definitions used across certified protocols.", | ||
| "defs": { | ||
| "did": { | ||
| "type": "object", | ||
| "description": "A Decentralized Identifier (DID) string.", | ||
| "required": ["did"], | ||
| "properties": { | ||
| "did": { | ||
| "type": "string", | ||
| "format": "did", | ||
| "description": "The DID string value.", | ||
| "maxLength": 256 | ||
| } | ||
|
Comment on lines
6
to
+16
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check for .changeset directory and look for relevant changesets
echo "=== Checking .changeset directory ==="
if [ -d .changeset ]; then
echo ".changeset directory found"
echo ""
echo "=== Contents of .changeset directory ==="
ls -la .changeset
echo ""
echo "=== Searching for changeset files mentioning DID or defs ==="
find .changeset -type f \( -name "*.md" -o -name "*.json" \) -exec grep -l "did\|defs\|DID" {} \; 2>/dev/null | head -20
echo ""
echo "=== Content of changeset files ==="
find .changeset -type f \( -name "*.md" \) ! -name "README*" -exec echo "File: {}" \; -exec cat {} \; 2>/dev/null
else
echo "No .changeset directory found"
fiRepository: hypercerts-org/hypercerts-lexicon Length of output: 24894 🏁 Script executed: #!/bin/bash
# Examine the defs.json file to understand the DID definition context
echo "=== Reading lexicons/app/certified/defs.json ==="
cat lexicons/app/certified/defs.json | head -100
echo ""
echo "=== Checking initial-version-post-repo-migration.md for DID details ==="
grep -A 5 -B 5 "did" .changeset/initial-version-post-repo-migration.mdRepository: hypercerts-org/hypercerts-lexicon Length of output: 1143 Expand the changeset to fully document the DID definition change. The DID definition in
Example format: 🤖 Prompt for AI Agents |
||
| } | ||
|
Comment on lines
+7
to
+17
There was a problem hiding this comment. The This is not directly caused by this PR, but the conversion highlights the issue. The field should probably be marked as optional (removed from the required array) if anonymity is intended to be supported. Consider whether this breaking change affects the intended semantics of the funding receipt schema.
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -116,20 +116,43 @@ | |||||||||
| } | ||||||||||
| }, | ||||||||||
| "contributorIdentity": { | ||||||||||
| "type": "object", | ||||||||||
| "description": "Contributor information as a string (DID or identifier).", | ||||||||||
| "required": ["identity"], | ||||||||||
| "properties": { | ||||||||||
| "identity": { | ||||||||||
| "type": "string", | ||||||||||
| "description": "The contributor identity string (DID or identifier).", | ||||||||||
| "maxLength": 1000, | ||||||||||
| "maxGraphemes": 100 | ||||||||||
| } | ||||||||||
| } | ||||||||||
| }, | ||||||||||
| "contributorRole": { | ||||||||||
| "type": "object", | ||||||||||
| "description": "Contribution details as a string.", | ||||||||||
| "required": ["role"], | ||||||||||
| "properties": { | ||||||||||
| "role": { | ||||||||||
| "type": "string", | ||||||||||
| "description": "The contribution role or details.", | ||||||||||
| "maxLength": 1000, | ||||||||||
|
There was a problem hiding this comment. The maxLength constraint has been significantly reduced from 10000 to 1000 (a 10x decrease). This is a breaking change that may cause existing valid data to be rejected. If this reduction is intentional due to the ATProto spec requirements for union types, it should be explicitly noted in the changeset and carefully considered for backward compatibility. Consider whether this stricter limit is necessary or if a less restrictive limit would still satisfy the requirements.
Suggested change
|
||||||||||
| "maxGraphemes": 100 | ||||||||||
|
Comment on lines
+139
to
+140
There was a problem hiding this comment. The maxLength and maxGraphemes constraints for the
Suggested change
|
||||||||||
| } | ||||||||||
|
cursor[bot] marked this conversation as resolved.
|
||||||||||
| } | ||||||||||
| }, | ||||||||||
| "workScopeString": { | ||||||||||
| "type": "object", | ||||||||||
| "description": "A free-form string describing the work scope for simple or legacy scopes.", | ||||||||||
| "required": ["scope"], | ||||||||||
| "properties": { | ||||||||||
| "scope": { | ||||||||||
| "type": "string", | ||||||||||
| "description": "The work scope description string.", | ||||||||||
| "maxLength": 1000, | ||||||||||
| "maxGraphemes": 100 | ||||||||||
|
aspiers marked this conversation as resolved.
|
||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a major bump for this breaking change.
Changing
app.certified.defs#didfrom a string to an object is a breaking change for consumers, so the changeset should use amajorbump.🔧 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents