From 7aac018f29d5e3ba7695aa3aeef88e99b284fcb4 Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Tue, 27 Jan 2026 10:15:16 +1300 Subject: [PATCH 1/6] feat: add validation unions types Without this patch, the lexicon style checker did not validate that union types only contain object or record type references. The ATProto spec requires that all union variants must be object or record types, e.g. primitive types like string, integer, and boolean are not allowed directly in unions. This is a problem because developers could accidentally create invalid lexicons with primitive types in unions, which would not conform to the ATProto specification. This patch solves the problem by: - Changing the validation logic to check for valid union types (object, record) - Reporting errors when a resolved type is NOT object or record - Adding a warning when a local ref resolves to a definition without a type - Clarifying that external refs cannot be validated without loading external lexicons - Improving error messages to focus on the requirement (must be object/record) rather than what's forbidden This makes the validation more aligned with the ATProto specification and more maintainable as it explicitly checks for valid types. Co-authored-by: Claude Code --- LEXICON_STYLE_GUIDE.md | 1 + scripts/check-lexicon-style.js | 126 ++++++++++++++++++++++++++++----- 2 files changed, 109 insertions(+), 18 deletions(-) diff --git a/LEXICON_STYLE_GUIDE.md b/LEXICON_STYLE_GUIDE.md index d446f855..0454fa90 100644 --- a/LEXICON_STYLE_GUIDE.md +++ b/LEXICON_STYLE_GUIDE.md @@ -183,6 +183,7 @@ The `style:check` script checks for: 8. ✅ Required fields are properly marked 9. ✅ StrongRef usage is documented 10. ✅ Lexicon IDs follow naming conventions +11. ✅ Union types only contain object or record types (no primitives) ## Running the Checker diff --git a/scripts/check-lexicon-style.js b/scripts/check-lexicon-style.js index a7cad585..97db9bff 100755 --- a/scripts/check-lexicon-style.js +++ b/scripts/check-lexicon-style.js @@ -98,6 +98,16 @@ class StyleChecker { const content = fs.readFileSync(filePath, "utf-8"); const lexicon = JSON.parse(content); + // Skip style checks for third-party and standard ATProto lexicons + if ( + lexicon.id && + (lexicon.id.startsWith("pub.leaflet.") || + lexicon.id.startsWith("app.bsky.") || + lexicon.id.startsWith("com.atproto.")) + ) { + return; + } + // Run all checks this.checkLexiconId(lexicon, fileResult); this.checkLexiconDescription(lexicon, fileResult); @@ -201,14 +211,14 @@ class StyleChecker { } for (const [defName, def] of Object.entries(lexicon.defs)) { - this.checkDefinition(def, `defs.${defName}`, fileResult); + this.checkDefinition(def, `defs.${defName}`, fileResult, lexicon); } } /** * Check a single definition */ - checkDefinition(def, path, fileResult) { + checkDefinition(def, path, fileResult, lexicon) { // Check for description if (!def.description) { fileResult.issues.push({ @@ -221,11 +231,11 @@ class StyleChecker { // Check based on type if (def.type === "record") { - this.checkRecordDefinition(def, path, fileResult); + this.checkRecordDefinition(def, path, fileResult, lexicon); } else if (def.type === "object") { - this.checkObjectDefinition(def, path, fileResult); + this.checkObjectDefinition(def, path, fileResult, lexicon); } else if (def.type === "array") { - this.checkArrayDefinition(def, path, fileResult); + this.checkArrayDefinition(def, path, fileResult, lexicon); } else if (def.type === "string") { this.checkStringDefinition(def, path, fileResult); } else if (def.type === "blob") { @@ -236,7 +246,7 @@ class StyleChecker { /** * Check record definition */ - checkRecordDefinition(def, path, fileResult) { + checkRecordDefinition(def, path, fileResult, lexicon) { // Check key type if (!def.key) { fileResult.issues.push({ @@ -256,14 +266,19 @@ class StyleChecker { // Check record schema if (def.record) { - this.checkObjectDefinition(def.record, `${path}.record`, fileResult); + this.checkObjectDefinition( + def.record, + `${path}.record`, + fileResult, + lexicon, + ); } } /** * Check object definition */ - checkObjectDefinition(obj, path, fileResult) { + checkObjectDefinition(obj, path, fileResult, lexicon) { if (!obj.properties) { return; } @@ -291,7 +306,12 @@ class StyleChecker { } // Check property type-specific rules - this.checkProperty(prop, `${path}.properties.${propName}`, fileResult); + this.checkProperty( + prop, + `${path}.properties.${propName}`, + fileResult, + lexicon, + ); } // Check for createdAt in main records @@ -313,17 +333,17 @@ class StyleChecker { /** * Check property-specific rules */ - checkProperty(prop, path, fileResult) { + checkProperty(prop, path, fileResult, lexicon) { if (prop.type === "string") { this.checkStringProperty(prop, path, fileResult); } else if (prop.type === "blob") { this.checkBlobProperty(prop, path, fileResult); } else if (prop.type === "array") { - this.checkArrayProperty(prop, path, fileResult); + this.checkArrayProperty(prop, path, fileResult, lexicon); } else if (prop.type === "ref") { this.checkRefProperty(prop, path, fileResult); } else if (prop.type === "union") { - this.checkUnionProperty(prop, path, fileResult); + this.checkUnionProperty(prop, path, fileResult, lexicon); } } @@ -405,7 +425,7 @@ class StyleChecker { /** * Check array property */ - checkArrayProperty(prop, path, fileResult) { + checkArrayProperty(prop, path, fileResult, lexicon) { if (!prop.items) { fileResult.issues.push({ severity: SEVERITY.ERROR, @@ -428,7 +448,7 @@ class StyleChecker { // Check items if (prop.items.type) { - this.checkProperty(prop.items, `${path}.items`, fileResult); + this.checkProperty(prop.items, `${path}.items`, fileResult, lexicon); } } @@ -463,10 +483,73 @@ class StyleChecker { } } + /** + * Resolve a local ref (starting with #) to its definition + */ + resolveLocalRef(ref, lexicon) { + if (typeof ref !== "string" || !ref.startsWith("#")) { + return null; + } + + const defName = ref.substring(1); // Remove the '#' + if (lexicon.defs && lexicon.defs[defName]) { + return lexicon.defs[defName]; + } + + return null; + } + + /** + * Check if a union ref points to a valid type (must be object or record) + */ + checkUnionRefForPrimitiveType(ref, index, path, fileResult, lexicon) { + const validUnionTypes = ["object", "record"]; + + // Check if this is an inline type definition + if (typeof ref === "object" && ref.type) { + if (!validUnionTypes.includes(ref.type)) { + fileResult.issues.push({ + severity: SEVERITY.ERROR, + rule: "union-invalid-type", + message: `Union variants must be object or record types. Inline type "${ref.type}" is not allowed in unions by ATProto spec`, + location: `${path}.refs[${index}]`, + }); + } + return; + } + + // Check if this is a local ref that we can resolve + if (typeof ref === "string" && ref.startsWith("#")) { + const resolvedDef = this.resolveLocalRef(ref, lexicon); + if (resolvedDef) { + if (!resolvedDef.type) { + fileResult.issues.push({ + severity: SEVERITY.WARNING, + rule: "union-invalid-type", + message: `Union variant local ref "${ref}" resolves to a definition without a type field`, + location: `${path}.refs[${index}]`, + }); + } else if (!validUnionTypes.includes(resolvedDef.type)) { + fileResult.issues.push({ + severity: SEVERITY.ERROR, + rule: "union-invalid-type", + message: `Union variants must be object or record types. Local ref "${ref}" resolves to type "${resolvedDef.type}" which is not allowed in unions by ATProto spec`, + location: `${path}.refs[${index}]`, + }); + } + } + return; + } + + // For external refs (e.g., "com.atproto.repo.strongRef" or "org.hypercerts.defs#uri"), + // we cannot validate without loading external lexicons, so we skip validation. + // These should be validated by the ATProto lexicon validator at build/runtime. + } + /** * Check union property */ - checkUnionProperty(prop, path, fileResult) { + checkUnionProperty(prop, path, fileResult, lexicon) { if (!prop.refs || prop.refs.length === 0) { fileResult.issues.push({ severity: SEVERITY.ERROR, @@ -474,9 +557,10 @@ class StyleChecker { message: "Union property must have refs array", location: `${path}.refs`, }); + return; } - if (prop.refs && prop.refs.length < 2) { + if (prop.refs.length < 2) { fileResult.issues.push({ severity: SEVERITY.INFO, rule: "union-multiple-types", @@ -484,6 +568,12 @@ class StyleChecker { location: `${path}.refs`, }); } + + // Check that all refs point to object or record types + // Primitive types (string, integer, boolean) are not allowed in unions + prop.refs.forEach((ref, index) => { + this.checkUnionRefForPrimitiveType(ref, index, path, fileResult, lexicon); + }); } /** @@ -503,8 +593,8 @@ class StyleChecker { /** * Check array definition */ - checkArrayDefinition(def, path, fileResult) { - this.checkArrayProperty(def, path, fileResult); + checkArrayDefinition(def, path, fileResult, lexicon) { + this.checkArrayProperty(def, path, fileResult, lexicon); } /** From e134b26c43a70c0a9ae04cc12b8a3bd05990c470 Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Tue, 27 Jan 2026 10:47:48 +1300 Subject: [PATCH 2/6] fix: convert union string types to object wrappers Without this patch, the contributorIdentity, contributorRole, and workScopeString definitions in org.hypercerts.claim.activity were defined as primitive string types, which violates the ATProto specification requirement that all union variants must be object or record types. This is a problem because the lexicon style checker now correctly detects and reports these violations as errors, and the lexicons would not be valid according to the ATProto spec. This patch solves the problem by: - Converting contributorIdentity from string to object with identity property - Converting contributorRole from string to object with role property - Converting workScopeString from string to object with scope property - Adding appropriate maxLength and maxGraphemes constraints to each property - Creating a changeset documenting the breaking changes Breaking changes: - Code using contributorIdentity must now access .identity property - Code using contributorRole must now access .role property - Code using workScopeString must now access .scope property Co-authored-by: Claude Code --- .changeset/fix-union-string-types.md | 27 ++++++++++++++ SCHEMAS.md | 18 ++++++++++ lexicons/org/hypercerts/claim/activity.json | 39 ++++++++++++++++----- 3 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 .changeset/fix-union-string-types.md diff --git a/.changeset/fix-union-string-types.md b/.changeset/fix-union-string-types.md new file mode 100644 index 00000000..0024dfca --- /dev/null +++ b/.changeset/fix-union-string-types.md @@ -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. diff --git a/SCHEMAS.md b/SCHEMAS.md index 400b83f5..2dd22872 100644 --- a/SCHEMAS.md +++ b/SCHEMAS.md @@ -41,6 +41,24 @@ Hypercerts-specific lexicons for tracking impact work and claims. | `contributionWeight` | `string` | ❌ | The relative weight/importance of this contribution (stored as a string to avoid float precision issues). Must be a positive numeric value. Weights do not need to sum to a specific total; normalization can be performed by the consuming application as needed. | | `contributionDetails` | `union` | ❌ | Contribution details as a string via org.hypercerts.claim.activity#contributorRole, or a strong reference to a contribution details record. | +##### `org.hypercerts.claim.activity#contributorIdentity` + +| Property | Type | Required | Description | +| ---------- | -------- | -------- | ---------------------------------------------------- | +| `identity` | `string` | ✅ | The contributor identity string (DID or identifier). | + +##### `org.hypercerts.claim.activity#contributorRole` + +| Property | Type | Required | Description | +| -------- | -------- | -------- | --------------------------------- | +| `role` | `string` | ✅ | The contribution role or details. | + +##### `org.hypercerts.claim.activity#workScopeString` + +| Property | Type | Required | Description | +| -------- | -------- | -------- | ---------------------------------- | +| `scope` | `string` | ✅ | The work scope description string. | + --- ### `org.hypercerts.claim.attachment` diff --git a/lexicons/org/hypercerts/claim/activity.json b/lexicons/org/hypercerts/claim/activity.json index 70d851a3..f914b47c 100644 --- a/lexicons/org/hypercerts/claim/activity.json +++ b/lexicons/org/hypercerts/claim/activity.json @@ -116,20 +116,43 @@ } }, "contributorIdentity": { - "type": "string", - "description": "Contributor information as a string (DID or identifier)." + "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": "string", + "type": "object", "description": "Contribution details as a string.", - "maxLength": 10000, - "maxGraphemes": 1000 + "required": ["role"], + "properties": { + "role": { + "type": "string", + "description": "The contribution role or details.", + "maxLength": 1000, + "maxGraphemes": 100 + } + } }, "workScopeString": { - "type": "string", + "type": "object", "description": "A free-form string describing the work scope for simple or legacy scopes.", - "maxLength": 10000, - "maxGraphemes": 1000 + "required": ["scope"], + "properties": { + "scope": { + "type": "string", + "description": "The work scope description string.", + "maxLength": 1000, + "maxGraphemes": 100 + } + } } } } From 24605ff01d9a10315708f3bef67a3228451d4793 Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Mon, 26 Jan 2026 22:07:45 +0000 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- scripts/check-lexicon-style.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/check-lexicon-style.js b/scripts/check-lexicon-style.js index 97db9bff..15ef08bb 100755 --- a/scripts/check-lexicon-style.js +++ b/scripts/check-lexicon-style.js @@ -537,6 +537,13 @@ class StyleChecker { location: `${path}.refs[${index}]`, }); } + } else { + fileResult.issues.push({ + severity: SEVERITY.WARNING, + rule: "union-unresolved-local-ref", + message: `Union variant local ref "${ref}" does not resolve to any definition in this lexicon (possible typo or missing definition)`, + location: `${path}.refs[${index}]`, + }); } return; } From a55df0409cef02f1c1d426335593f9047001f29d Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Tue, 27 Jan 2026 11:28:13 +1300 Subject: [PATCH 4/6] feat: add external ref resolution and validation Without this patch, the union type validator only checked local refs (starting with #) and could not validate external refs like "org.hypercerts.defs#smallImage" or "com.atproto.repo.strongRef". This meant that typos or invalid external refs would not be caught by the style checker. This is a problem because developers could reference non-existent external definitions or reference definitions that resolve to invalid types (like primitives), and these errors would only be caught at runtime or by the ATProto lexicon validator. This patch solves the problem by: - Adding buildLexiconIndex() to load all lexicons into a Map for lookup - Adding resolveExternalRef() to resolve external refs to their definitions - Enhancing checkUnionRefForPrimitiveType() to validate external refs - Reporting errors for unresolved refs (possible typos or missing lexicons) - Reporting errors when external refs resolve to non-object/record types The checker now validates all three types of union refs: 1. Inline definitions (e.g., {type: "string"}) 2. Local refs (e.g., "#contributorIdentity") 3. External refs (e.g., "org.hypercerts.defs#uri") This enhancement caught a real issue: app.certified.defs#did was a string type being used in a union, which violates the ATProto spec. Co-authored-by: Claude Code --- scripts/check-lexicon-style.js | 77 ++++++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/scripts/check-lexicon-style.js b/scripts/check-lexicon-style.js index 15ef08bb..d2fc9925 100755 --- a/scripts/check-lexicon-style.js +++ b/scripts/check-lexicon-style.js @@ -46,12 +46,35 @@ class StyleChecker { warningCount: 0, infoCount: 0, }; + this.lexiconIndex = new Map(); // Map of lexicon ID to parsed lexicon + } + + /** + * Load all lexicons into an index for ref resolution + */ + buildLexiconIndex(dir) { + const files = this.findLexiconFiles(dir); + + for (const file of files) { + try { + const content = fs.readFileSync(file, "utf-8"); + const lexicon = JSON.parse(content); + if (lexicon.id) { + this.lexiconIndex.set(lexicon.id, lexicon); + } + } catch (error) { + // Skip files that can't be parsed - they'll be caught in checkFile + } + } } /** * Check all lexicon files in a directory */ async checkDirectory(dir) { + // First, build an index of all lexicons + this.buildLexiconIndex(dir); + const files = this.findLexiconFiles(dir); for (const file of files) { @@ -499,6 +522,27 @@ class StyleChecker { return null; } + /** + * Resolve an external ref (e.g., "org.hypercerts.defs#uri") to its definition + */ + resolveExternalRef(ref) { + if (typeof ref !== "string" || ref.startsWith("#")) { + return null; + } + + // Parse the ref: "lexicon.id#defName" or just "lexicon.id" + const parts = ref.split("#"); + const lexiconId = parts[0]; + const defName = parts[1] || "main"; + + const targetLexicon = this.lexiconIndex.get(lexiconId); + if (!targetLexicon || !targetLexicon.defs) { + return null; + } + + return targetLexicon.defs[defName] || null; + } + /** * Check if a union ref points to a valid type (must be object or record) */ @@ -548,9 +592,36 @@ class StyleChecker { return; } - // For external refs (e.g., "com.atproto.repo.strongRef" or "org.hypercerts.defs#uri"), - // we cannot validate without loading external lexicons, so we skip validation. - // These should be validated by the ATProto lexicon validator at build/runtime. + // Check if this is an external ref (e.g., "com.atproto.repo.strongRef" or "org.hypercerts.defs#uri") + if (typeof ref === "string" && !ref.startsWith("#")) { + const resolvedDef = this.resolveExternalRef(ref); + if (resolvedDef) { + if (!resolvedDef.type) { + fileResult.issues.push({ + severity: SEVERITY.WARNING, + rule: "union-invalid-type", + message: `Union variant external ref "${ref}" resolves to a definition without a type field`, + location: `${path}.refs[${index}]`, + }); + } else if (!validUnionTypes.includes(resolvedDef.type)) { + fileResult.issues.push({ + severity: SEVERITY.ERROR, + rule: "union-invalid-type", + message: `Union variants must be object or record types. External ref "${ref}" resolves to type "${resolvedDef.type}" which is not allowed in unions by ATProto spec`, + location: `${path}.refs[${index}]`, + }); + } + } else { + // Could not resolve the external ref - might be a typo or the lexicon might not be loaded + fileResult.issues.push({ + severity: SEVERITY.ERROR, + rule: "union-unresolved-ref", + message: `Union variant external ref "${ref}" cannot be resolved (possible typo or missing lexicon)`, + location: `${path}.refs[${index}]`, + }); + } + return; + } } /** From da481e09f5bd1a8e62e388f2c6001896d76b1fbf Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Tue, 27 Jan 2026 11:28:31 +1300 Subject: [PATCH 5/6] fix: convert app.certified.defs#did to object type Without this patch, the app.certified.defs#did definition was a primitive string type, which violates the ATProto specification when used in a union. The app.certified.badge.award lexicon uses this definition in a union type for the subject property, which should only contain object or record types. This is a problem because the enhanced style checker now correctly detects and reports this as an error, and the lexicon would not be valid according to the ATProto spec. This patch solves the problem by: - Converting the did definition from type: string to type: object - Wrapping the DID string value in a required 'did' property - Adding maxLength constraint of 256 (appropriate for DID strings) - Preserving the format: did constraint on the inner string property Breaking change: - Code using app.certified.defs#did must now access the .did property instead of using the value directly Co-authored-by: Claude Code --- .changeset/fix-certified-did-type.md | 17 +++++++++++++++++ SCHEMAS.md | 8 ++++++++ lexicons/app/certified/defs.json | 14 +++++++++++--- 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 .changeset/fix-certified-did-type.md diff --git a/.changeset/fix-certified-did-type.md b/.changeset/fix-certified-did-type.md new file mode 100644 index 00000000..a20b99c0 --- /dev/null +++ b/.changeset/fix-certified-did-type.md @@ -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 diff --git a/SCHEMAS.md b/SCHEMAS.md index 2dd22872..417e9d03 100644 --- a/SCHEMAS.md +++ b/SCHEMAS.md @@ -360,6 +360,14 @@ Certified lexicons are common/shared lexicons that can be used across multiple p **Description:** Common type definitions used across certified protocols. +#### Defs + +##### `app.certified.defs#did` + +| Property | Type | Required | Description | +| -------- | -------- | -------- | --------------------- | +| `did` | `string` | ✅ | The DID string value. | + --- ## Type Definitions diff --git a/lexicons/app/certified/defs.json b/lexicons/app/certified/defs.json index 81d39590..a808ff5e 100644 --- a/lexicons/app/certified/defs.json +++ b/lexicons/app/certified/defs.json @@ -4,9 +4,17 @@ "description": "Common type definitions used across certified protocols.", "defs": { "did": { - "type": "string", - "format": "did", - "description": "A Decentralized Identifier (DID) string." + "type": "object", + "description": "A Decentralized Identifier (DID) string.", + "required": ["did"], + "properties": { + "did": { + "type": "string", + "format": "did", + "description": "The DID string value.", + "maxLength": 256 + } + } } } } From 9bd413bb0b2ce9ae2968b1a4d0c1e5bbe7280b1a Mon Sep 17 00:00:00 2001 From: Adam Spiers Date: Tue, 27 Jan 2026 11:28:51 +1300 Subject: [PATCH 6/6] ai: allow running style:check --- opencode.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opencode.json b/opencode.json index 4c8dd7c7..66d85267 100644 --- a/opencode.json +++ b/opencode.json @@ -22,7 +22,7 @@ "npm run list": "allow", "npm run prepublishOnly": "deny", "npm run release": "deny", - "npm run style": "allow", + "npm run style:check": "allow", "npm run test": "allow", "npm run test:watch": "allow", "npm run typecheck": "allow",