feat: add Artifacts binding TypeScript definitions#6593
Conversation
Add type definitions for the Artifacts binding to types/defines/. Defines the Artifacts interface with methods for: - Repository management: createRepo, getRepo, listRepos, deleteRepo - Fork operations: forkRepo, importFromGitHub - Token management: createToken, validateToken, listTokens, revokeToken All methods include JSDoc documentation with parameter descriptions. Ref: cloudflare/ai-agents/artifacts#2
- Add missing fields to ArtifactsRepoInfo: description, defaultBranch, updatedAt, lastPushAt - Add missing fields to ArtifactsCreateRepoResult: description, defaultBranch - Fix ArtifactsRepoListResult.repos to use Omit<ArtifactsRepoInfo, 'remote'> since list() does not include remote URLs - Fix token scope values from 'rw'/'r' to 'write'/'read' to match the public Scope type - Add missing 'state' field to ArtifactsTokenInfo - Update JSDoc comments to reflect correct scope values
Add Artifacts binding types to the generated snapshot files (index.d.ts and index.ts) to match bazel-generated output.
- fork(): flatten to (name, opts?) with description, readOnly, defaultBranchOnly
- create(): add description and setDefaultBranch to opts
- import(): restructure to params object with source and target
- Flatten return types: remove '& { repo: ArtifactsRepo }' intersections
- get(): add JSDoc note about transient states (import/fork in progress)
- Update all 4 generated snapshot files to match
- Remove Artifacts.import() method entirely
- Add ArtifactsGetRepoResult type with repo handle and status field
- get() now returns { repo, status } where status is 'ready' | 'importing' | 'forking'
- Update all generated snapshots
ArtifactsGetRepoResult is now:
| { status: 'ready'; repo: ArtifactsRepo }
| { status: 'not_found' }
| { status: 'importing'; retryAfter: number }
| { status: 'forking'; retryAfter: number }
- Rename expiresAt → tokenExpiresAt on ArtifactsCreateRepoResult - Remove ArtifactsTokenValidation and validateToken() - Remove ArtifactsGetRepoResult union, get() returns ArtifactsRepo directly - Inline repo fields directly on ArtifactsRepo (no more info() method) - Restore import() with source/target params structure - Update all generated snapshots
Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR adds hand-written TypeScript type definitions for the new Artifacts binding (repo management, token management, fork operations, import) and updates the generated snapshots.
Issues
-
ArtifactsTokenListResultis missingcursorfor pagination —ArtifactsRepoListResultsupports cursor-based pagination with an optionalcursorfield, but the structurally-analogousArtifactsTokenListResultonly hastokensandtotal. If the token list endpoint supports pagination (or will in the future), this field should be present for consistency. Similarly,listTokens()takes no arguments, so there's no way to passlimit/cursoreven if the API supports it. -
listTokens()has no pagination parameters — Related to (1):Artifacts.list()acceptsopts?: { limit?: number; cursor?: string }butArtifactsRepo.listTokens()takes zero arguments. If a repo can accumulate many tokens, callers have no way to paginate. If the backing API genuinely doesn't support pagination here, it's fine to leave as-is, but please confirm this is intentional.
| tokens: ArtifactsTokenInfo[]; | ||
| /** Total number of tokens for the repository. */ | ||
| total: number; | ||
| } |
There was a problem hiding this comment.
ArtifactsRepoListResult has a cursor field for pagination, but the analogous ArtifactsTokenListResult does not. If the token-list API supports (or will support) pagination, this should be consistent:
| } | |
| interface ArtifactsTokenListResult { | |
| /** Tokens in this page. */ | |
| tokens: ArtifactsTokenInfo[]; | |
| /** Total number of tokens for the repository. */ | |
| total: number; | |
| /** Cursor for the next page, if there are more results. */ | |
| cursor?: string; | |
| } |
If the backing API genuinely returns all tokens in one shot with no pagination, this is fine as-is — just confirming it's intentional.
| /** List tokens for this repo (metadata only, no plaintext). */ | ||
| listTokens(): Promise<ArtifactsTokenListResult>; |
There was a problem hiding this comment.
Same question: Artifacts.list() accepts { limit?, cursor? } but listTokens() takes no pagination opts. If the token-list endpoint supports pagination, this should accept similar options:
| /** List tokens for this repo (metadata only, no plaintext). */ | |
| listTokens(): Promise<ArtifactsTokenListResult>; | |
| /** List tokens for this repo (metadata only, no plaintext). */ | |
| listTokens(opts?: { limit?: number; cursor?: string }): Promise<ArtifactsTokenListResult>; |
|
I'm Bonk, and I've done a quick review of your PR. This PR adds hand-written TypeScript type definitions for the new Artifacts binding (repo management, token management, fork operations, import) and updates the generated snapshots. The types follow the I posted two related comments about a pagination inconsistency:
Both comments include suggestion blocks. If the backing API genuinely doesn't paginate tokens, these are fine to dismiss -- just wanted confirmation it's intentional. |
Merging this PR will not alter performance
Comparing Footnotes
|
|
Closing since this PR served its purpose (I believe) |
Summary
Add TypeScript type definitions for the Artifacts binding
Changes
New file:
types/defines/artifacts.d.tsNote
This PR is a copy of #6508 in the hope to get the internal-build workflow to pass