fix(deps): remove dependency on @atproto/api
#1295
Conversation
matthieusieben
force-pushed
the
msi/atproto-integration
branch
from
947b270 to
ea8ba3f
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR removes the Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
danielroe
changed the title
@atproto/api@atproto/api
matthieusieben
force-pushed
the
msi/atproto-integration
branch
from
ea8ba3f to
95b09f4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/api/atproto/bluesky-comments.get.ts (1)
60-63:⚠️ Potential issue | 🟡 MinorAdd an explicit array length guard before index access.
Line 62 indexespostsResponse.posts?.[0]without a length check. Use an explicit guard to keep access strictly type‑safe.Suggested fix
const thread = parseThread(threadResponse.thread) + const firstPost = postsResponse.posts?.length ? postsResponse.posts[0] : undefined return { thread, likes: likesResponse.likes, - totalLikes: postsResponse.posts?.[0]?.likeCount ?? thread?.likeCount ?? 0, + totalLikes: firstPost?.likeCount ?? thread?.likeCount ?? 0, postUrl: atUriToWebUrl(uri), }As per coding guidelines, "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
| "data": { | ||
| "type": "string", | ||
| "maxLength": 3000, | ||
| "description": "Arbitrary data for the NUX. The structure is defined by the NUX itself. Limited to 300 characters.", | ||
| "maxGraphemes": 300 |
There was a problem hiding this comment.
Clarify the NUX data length limit.
The description says “Limited to 300 characters” while maxLength is 3000 and maxGraphemes is 300. Please align the text with the intended constraint to avoid ambiguity.
Suggested fix (doc‑only clarification)
- "description": "Arbitrary data for the NUX. The structure is defined by the NUX itself. Limited to 300 characters.",
+ "description": "Arbitrary data for the NUX. The structure is defined by the NUX itself. Limited to 300 graphemes (maxLength 3000).",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "data": { | |
| "type": "string", | |
| "maxLength": 3000, | |
| "description": "Arbitrary data for the NUX. The structure is defined by the NUX itself. Limited to 300 characters.", | |
| "maxGraphemes": 300 | |
| "data": { | |
| "type": "string", | |
| "maxLength": 3000, | |
| "description": "Arbitrary data for the NUX. The structure is defined by the NUX itself. Limited to 300 graphemes (maxLength 3000).", | |
| "maxGraphemes": 300 |
| "referencelist": { | ||
| "type": "token", | ||
| "description": "A list of actors used for only for reference purposes such as within a starter pack." | ||
| }, |
There was a problem hiding this comment.
Fix typo: duplicated "for" in description.
The description contains "used for only for reference purposes" which should be "used only for reference purposes".
📝 Proposed fix
"referencelist": {
"type": "token",
- "description": "A list of actors used for only for reference purposes such as within a starter pack."
+ "description": "A list of actors used only for reference purposes such as within a starter pack."
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "referencelist": { | |
| "type": "token", | |
| "description": "A list of actors used for only for reference purposes such as within a starter pack." | |
| }, | |
| "referencelist": { | |
| "type": "token", | |
| "description": "A list of actors used only for reference purposes such as within a starter pack." | |
| }, |
| "reasonChildSafetyCSAM": { | ||
| "type": "token", | ||
| "description": "Child sexual abuse material (CSAM). These reports will be sent only be sent to the application's Moderation Authority." | ||
| }, |
There was a problem hiding this comment.
Fix grammatical error in descriptions: "will be sent only be sent" → "will only be sent".
Several reason descriptions contain a duplicated phrase. This typo appears in:
- Line 115:
reasonChildSafetyCSAM - Line 143:
reasonChildSafetyGroom - Line 147:
reasonChildSafetyOther - Line 207:
reasonViolenceExtremistContent
📝 Proposed fix
"reasonChildSafetyCSAM": {
"type": "token",
- "description": "Child sexual abuse material (CSAM). These reports will be sent only be sent to the application's Moderation Authority."
+ "description": "Child sexual abuse material (CSAM). These reports will only be sent to the application's Moderation Authority."
}, "reasonChildSafetyGroom": {
"type": "token",
- "description": "Grooming or predatory behavior. These reports will be sent only be sent to the application's Moderation Authority."
+ "description": "Grooming or predatory behavior. These reports will only be sent to the application's Moderation Authority."
},
"reasonChildSafetyOther": {
"type": "token",
- "description": "Other child safety. These reports will be sent only be sent to the application's Moderation Authority."
+ "description": "Other child safety. These reports will only be sent to the application's Moderation Authority."
}, "reasonViolenceExtremistContent": {
"type": "token",
- "description": "Extremist content. These reports will be sent only be sent to the application's Moderation Authority."
+ "description": "Extremist content. These reports will only be sent to the application's Moderation Authority."
}Also applies to: 141-148, 205-208
There was a problem hiding this comment.
The lexicon documents from app.bsky, com.atproto, chat.bsky and tools.ozone namespaces are fetched as dependencies and should not be altered here.
There was a problem hiding this comment.
@matthieusieben, thank you for the clarification! I understand now that these lexicon documents are external dependencies and shouldn't be modified here. I'll make note of this for future reviews.
✏️ Learnings added
Learnt from: matthieusieben
Repo: npmx-dev/npmx.dev PR: 1295
File: lexicons/tools/ozone/report/defs.json:113-116
Timestamp: 2026-02-09T20:02:54.065Z
Learning: Lexicon documents from the app.bsky, com.atproto, chat.bsky, and tools.ozone namespaces in the npmx.dev project are fetched as dependencies and should not be altered or have issues flagged in code reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: matthieusieben
Repo: npmx-dev/npmx.dev PR: 1295
File: lexicons.json:3-13
Timestamp: 2026-02-09T19:59:28.674Z
Learning: In the npmx.dev project's lexicons.json file, the "lexicons" array should only contain explicitly referenced lexicon namespaces that are used directly in the code. Other lexicon documents are dependencies and should appear in the "resolutions" section but do not need to be listed in the "lexicons" array.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
fatfingers23
left a comment
fatfingers23
left a comment
There was a problem hiding this comment.
LGTM! Thank you for the PR.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/api/auth/atproto.get.ts (1)
143-144:⚠️ Potential issue | 🟡 MinorAdd validation for external API response using the existing schema.
The JSON response at line 144 is directly cast to
PublicUserSessionwithout validation. APublicUserSessionSchemausing Valibot already exists inshared/schemas/publicUserSession.ts, and the validation pattern is established elsewhere in the codebase (seeserver/api/auth/session.get.ts).Import and use
PublicUserSessionSchemawithsafeParse()to validate the response before casting, matching the validation pattern used for other data sources:import { PublicUserSessionSchema } from '#shared/schemas/publicUserSession' import { safeParse } from 'valibot' // At line 144: const parseResult = safeParse(PublicUserSessionSchema, await response.json()) if (!parseResult.success) { // Handle validation error } const miniDoc: PublicUserSession = parseResult.output
🧹 Nitpick comments (2)
server/api/auth/atproto.get.ts (1)
25-33: Redundant truthiness check after validation.After the early return when
isAtIdentifierString(did)is false (lines 25-27), thedid &&check on line 33 is redundant. A valid AT identifier string cannot be empty or falsy.Proposed simplification
if (did && pdsUrl.protocol === 'https:') { + if (pdsUrl.protocol === 'https:') {server/api/atproto/bluesky-comments.get.ts (1)
80-85: Add type guards for regex capture groups to ensure type safety.When destructuring
match, TypeScript typesdidandrkeyasstring | undefinedeven though logically the regex guarantees they exist whenmatchis non-null. As per coding guidelines, ensure strictly type-safe code by checking when accessing values.🛡️ Proposed fix for stricter type safety
function atUriToWebUrl(uri: AtUriString): string | null { const match = uri.match(AT_URI_REGEX) - if (!match) return null - const [, did, rkey] = match + if (!match?.[1] || !match[2]) return null + const did = match[1] + const rkey = match[2] return `https://bsky.app/profile/${did}/post/${rkey}` }
|
Deployment failed with the following error: |
| "signing_key": { | ||
| "type": "string", | ||
| "description": "The atproto signing key publicKeyMultibase\n\nLegacy key encoding not supported. the key is returned directly; `id`,\n`type`, and `controller` are omitted." | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
AT Protocol lexicon property naming conventions camelCase
💡 Result:
In AT Protocol Lexicon, property/field names should use lower camelCase (aka lowerCamelCase)—not UpperCamelCase, snake_case, or ALL_CAPS. [1]
Additional relevant rules from the Lexicon style guide:
- Allowed characters (same as NSID name segments): ASCII alphanumeric, case-sensitive, must not start with a digit, and no hyphens. [1]
- Do not use
$-prefixed field names anywhere in objects; they’re reserved for future protocol-level extensions. [1] - Related naming conventions: API error names use
UpperCamelCase, and fixed string values (e.g.,knownValues) use kebab-case. [1]
Sources:
[1] AT Protocol – Lexicon Style Guide (atproto.com)
Citations:
🏁 Script executed:
# Find and examine the specific file
find . -name "resolveMiniDoc.json" -type fRepository: npmx-dev/npmx.dev
Length of output: 116
🏁 Script executed:
# Look for other lexicon files to see naming patterns
find . -path "*/lexicons/*.json" -type f | head -20Repository: npmx-dev/npmx.dev
Length of output: 841
🏁 Script executed:
# Examine the specific file
cat -n ./lexicons/com/bad-example/identity/resolveMiniDoc.jsonRepository: npmx-dev/npmx.dev
Length of output: 2175
🏁 Script executed:
# Check some existing Bluesky lexicon files to see naming patterns
cat -n ./lexicons/app/bsky/actor/defs.json | head -100Repository: npmx-dev/npmx.dev
Length of output: 3579
Rename signing_key to signingKey to comply with AT Protocol naming conventions.
AT Protocol Lexicon style guide mandates that property names use lower camelCase. The property should be signingKey rather than signing_key.
Required change
- "signing_key": {
+ "signingKey": {
"type": "string",
"description": "The atproto signing key publicKeyMultibase\n\nLegacy key encoding not supported. the key is returned directly; `id`,\n`type`, and `controller` are omitted."
}Also update line 9 to reflect the change in the required array.
There was a problem hiding this comment.
com.bad-example is not a namespace managed from this repo. That file was downloaded.
There was a problem hiding this comment.
@matthieusieben, you're absolutely right—I apologize for the incorrect suggestion. Since com.bad-example is a downloaded namespace and not managed by this repository, the file should not be modified. I'll remember that for future reviews.
✏️ Learnings added
Learnt from: matthieusieben
Repo: npmx-dev/npmx.dev PR: 1295
File: lexicons/com/bad-example/identity/resolveMiniDoc.json:26-29
Timestamp: 2026-02-09T20:36:23.557Z
Learning: The `com.bad-example` namespace in the npmx.dev project is fetched as a dependency and should not be altered or have issues flagged in code reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: matthieusieben
Repo: npmx-dev/npmx.dev PR: 1295
File: lexicons/tools/ozone/report/defs.json:113-116
Timestamp: 2026-02-09T20:02:54.065Z
Learning: Lexicon documents from the app.bsky, com.atproto, chat.bsky, and tools.ozone namespaces in the npmx.dev project are fetched as dependencies and should not be altered or have issues flagged in code reviews.
Learnt from: matthieusieben
Repo: npmx-dev/npmx.dev PR: 1295
File: lexicons.json:3-13
Timestamp: 2026-02-09T19:59:28.674Z
Learning: In the npmx.dev project's lexicons.json file, the "lexicons" array should only contain explicitly referenced lexicon namespaces that are used directly in the code. Other lexicon documents are dependencies and should appear in the "resolutions" section but do not need to be listed in the "lexicons" array.
server/api/auth/atproto.get.ts
Outdated
| const client = new Client({ service: SLINGSHOT_HOST }) | ||
| const response = await client.xrpcSafe(com['bad-example'].identity.resolveMiniDoc, { | ||
| headers: { 'User-Agent': 'npmx' }, | ||
| params: { identifier: authSession.did }, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file in question
fd "atproto.get.ts" --type fRepository: npmx-dev/npmx.dev
Length of output: 91
🏁 Script executed:
# Check the SLINGSHOT_HOST constant definition
rg "SLINGSHOT_HOST" --type ts --type tsx -B 2 -A 2Repository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Look at the `@atproto/lex` package to understand service URL format
fd "package.json" --type f | head -1 | xargs cat | grep -A 5 -B 5 "@atproto/lex" || rg "@atproto/lex" --type jsonRepository: npmx-dev/npmx.dev
Length of output: 103
🏁 Script executed:
# Read the file to see the code context
cat -n server/api/auth/atproto.get.ts | sed -n '130,150p'Repository: npmx-dev/npmx.dev
Length of output: 831
🏁 Script executed:
# Search for SLINGSHOT_HOST definition
rg "SLINGSHOT_HOST" -B 2 -A 2Repository: npmx-dev/npmx.dev
Length of output: 1682
🏁 Script executed:
# Check node_modules or documentation for `@atproto/lex` Client
fd "node_modules/@atproto/lex" -type d 2>/dev/null | head -1 | xargs ls -la 2>/dev/null || echo "Checking alternative locations"Repository: npmx-dev/npmx.dev
Length of output: 3680
🏁 Script executed:
# Search for other Client usages in the codebase to see the pattern
rg "new Client" -B 2 -A 2Repository: npmx-dev/npmx.dev
Length of output: 2389
🏁 Script executed:
# Check if there are any other service URL configurations
rg "service:" -B 1 -A 1 | head -30Repository: npmx-dev/npmx.dev
Length of output: 711
🌐 Web query:
@atproto/lex Client service URL format required scheme
💡 Result:
In @atproto/* Lex/XRPC clients, the service (aka base/service URL) must be a fully-qualified URL that includes a scheme, i.e. start with https:// (or http:// for non-TLS/dev). Passing something like bsky.social (no scheme) will fail because the client builds request URLs using the platform URL parser (new URL(...)), which requires an absolute base URL. [1][2]
Valid examples
https://bsky.socialhttps://public.api.bsky.apphttp://localhost:2583(dev)
Invalid
bsky.social(missing scheme) [1][2]
Format: https://<host>[:port] (the client will append /xrpc/<nsid> internally). [1]
Sources: [1] [2]
🏁 Script executed:
# Check what BLUESKY_API is defined as
rg "BLUESKY_API" --type ts -B 1 | head -20Repository: npmx-dev/npmx.dev
Length of output: 831
🏁 Script executed:
# Check shared/utils/constants.ts to see all constants
cat -n shared/utils/constants.ts | head -50Repository: npmx-dev/npmx.dev
Length of output: 2935
Add HTTPS scheme to the SLINGSHOT_HOST constant.
SLINGSHOT_HOST is defined as 'slingshot.microcosm.blue' without a scheme. The @atproto/lex Client requires a fully-qualified URL with a scheme (e.g., https://), as it uses the platform URL parser internally. Without it, the client will fail to build valid XRPC request URLs and crash during the callback flow.
Update the constant in shared/utils/constants.ts to include the scheme, consistent with how BLUESKY_API is defined:
Suggested fix
-export const SLINGSHOT_HOST = 'slingshot.microcosm.blue'
+export const SLINGSHOT_HOST = 'https://slingshot.microcosm.blue'
|
Thanks for your first contribution, @matthieusieben! 💥 We'd love to welcome you to the npmx community. Come and say hi on Discord! And once you've joined, visit npmx.wamellow.com to claim the contributor role. |
3 participants
@atproto/apiand@atproto/lexare redundent system for working with AT Protocol data, with@atproto/lexbeing more recent.This PR replaces the use of
@atproto/apiwith@atproto/lex.