Repository navigation
feat!: Add create-actor and update-actor-version tools #1453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
DaveHanns
wants to merge
121
commits into
master
Choose a base branch
from
feat/update-actor-version
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+5,821
−226
Open
Changes from 94 commits
Commits
Show all changes
121 commits
Select commit
Hold shift + click to select a range
6ba95b7
feat: Add get-actor-version tool in a new source category
DaveHanns f45d08d
fix: Harden get-actor-version zip reading, inline files, and hints
DaveHanns 6851981
docs: Link the current API pages in get-actor-version
DaveHanns a27395e
feat: Add create-actor and update-actor-version tools
DaveHanns 534127d
fix: Harden create-actor and update-actor-version writes
DaveHanns f398803
refactor: Share file input parsing between source write tools
DaveHanns 83951fd
docs: Mark where get-actor-version reads a whole source
DaveHanns 8f297a1
Merge remote-tracking branch 'origin/feat/get-actor-version' into fea…
DaveHanns e2d7e8a
docs: Mark where update-actor-version writes a whole source
DaveHanns b9df355
refactor: Refuse zip-stored versions in get-actor-version and simplif…
DaveHanns 24f9792
Merge branch 'feat/get-actor-version' into feat/update-actor-version
DaveHanns aa2d8fe
refactor: Share the manifest builder and align the zip refusal
DaveHanns 8f2f928
refactor: Tidy get-actor-version after dropping zip reading
DaveHanns fa1c1b9
Merge branch 'feat/get-actor-version' into feat/update-actor-version
DaveHanns 2f77449
refactor: Decode stored source entries in one place
DaveHanns 4daa78b
refactor: Simplify get-actor-version
DaveHanns 1b28077
refactor: Address review of the get-actor-version simplification
DaveHanns 46a53c3
Merge branch 'feat/get-actor-version' into feat/update-actor-version
DaveHanns 1c7bcab
refactor: Keep the write tools' source helpers after the get-actor-ve…
DaveHanns 0519a95
refactor: Simplify create-actor and update-actor-version
DaveHanns 639cffc
refactor: Address review of the create-actor and update-actor-version…
DaveHanns e489d3c
Merge remote-tracking branch 'origin/master' into feat/get-actor-version
DaveHanns 0ecc220
Merge branch 'feat/get-actor-version' into feat/update-actor-version
DaveHanns b9e02ca
refactor: Report only API errors from the build start as buildError
DaveHanns f3da605
Merge remote-tracking branch 'origin/master' into feat/get-actor-version
DaveHanns 5650f16
Merge branch 'feat/get-actor-version' into feat/update-actor-version
DaveHanns 48f8db4
test: Cover read-only calls, env var leaks, and sub-resources in get-…
DaveHanns 749d1f3
test: Cover exact content, line range edges, and the byte limit in ge…
DaveHanns 8b22d9d
test: Cover hashes, revisions, path order, and URL redaction in get-a…
DaveHanns 20b4796
test: Cover the text block, edge-case files, and the 256 KiB limit in…
DaveHanns c47b869
Merge branch 'feat/get-actor-version' into feat/update-actor-version
DaveHanns 86fac27
test: Expect the shared refusal wording in the new get-actor-version …
DaveHanns 11a6247
test: Cover untouched files, the PUT body, operation order, and refus…
DaveHanns 44a3b59
test: Cover the POST body, file mapping, cancellation, and autoBuild …
DaveHanns 9a88b74
test: Cover empty files in the PUT, multi-edit, whole-value hash chec…
DaveHanns 49497d5
test: Cover a cancel during the build start in update-actor-version
DaveHanns 704f329
refactor: Address review nits in get-actor-version
DaveHanns 41285bc
fix: Name left-out and missing files in the get-actor-version summary
DaveHanns bae722b
fix: Normalize requested paths in get-actor-version
DaveHanns 1b372e9
fix: Explain in plain words why get-actor-version refuses a version
DaveHanns 0a183a5
test: Merge near-duplicate get-actor-version tests into it.each
DaveHanns 5f6f54e
Merge branch 'feat/get-actor-version' into feat/update-actor-version
DaveHanns 95785dd
refactor: Read the eager file content in update-actor-version
DaveHanns ea9a5e7
test: Expect the new refusal wording in update-actor-version
DaveHanns b25a51a
style: Format the refusal tests in update-actor-version
DaveHanns ea000a6
Merge remote-tracking branch 'origin/master' into feat/update-actor-v…
DaveHanns 1ac3793
test: Cover stored names, the input schema, and more cases of update-…
DaveHanns 379e254
test: Cover the input schema, Console link, and non-ASCII paths of cr…
DaveHanns 72413ad
test: Check that get-actor-version names the Actor by its document
DaveHanns 824b2a4
fix: Count overlapping matches of an oldText as several
DaveHanns 5e479d1
fix: Refuse to write a file outside the Actor root or at a folder path
DaveHanns 4c310fa
fix: Refuse a new file whose path collides with a file or folder
DaveHanns 22d7709
fix: Refuse two files at the same path in create-actor
DaveHanns 43823d7
fix: Check the expectedHash an edit operation carries
DaveHanns 2ff9fb1
fix: Refuse content the stored file would not hold as sent
DaveHanns f82051c
fix: Report any failed build start after a write as buildError
DaveHanns 94c660a
fix: Keep line endings CRLF when an LF edit changes a CRLF file
DaveHanns cfd8a4d
fix: Say that an Actor has no versions instead of listing none
DaveHanns 32eed8c
test: Run create-actor, get-actor-version, and update-actor-version a…
DaveHanns dec4503
test: Add live integration cases for create-actor and update-actor-ve…
DaveHanns f4768c3
test: Remove the second live case for the source tools from actors.ca…
DaveHanns 6cf8057
test: Seed entries without format or content in the live update case
DaveHanns b8cdc79
test: Check that update-actor-version names no deleted empty file as …
DaveHanns 2a51ce4
test: Check that a folder stored under an unnormalized name blocks a …
DaveHanns 3668c69
fix: Say that a retried create-actor POST may have created the Actor
DaveHanns f06edcd
Revert "fix: Say that an Actor has no versions instead of listing none"
DaveHanns 7387891
Merge remote-tracking branch 'origin/master' into feat/update-actor-v…
DaveHanns 986367d
fix: Name get-actor-version in the retried create-actor message only …
DaveHanns 8eee7c8
fix: Say to build the version before running it, not to run the files
DaveHanns 9cc64d9
fix: Say create-actor uploads new Actor code, not publishes it
DaveHanns 88a8173
docs: Make the Actor source feature line one plain list
DaveHanns 2774c2c
Merge remote-tracking branch 'origin/master' into feat/update-actor-v…
DaveHanns 90737fb
fix: Limit update-actor-version to 100 operations and 100 edits per call
DaveHanns cf835c2
fix: Keep file content out of the logged arguments of the source tools
DaveHanns 163d817
fix: Refuse control characters in source paths and encode the revisio…
DaveHanns 9930b52
fix: Start no build after a source write when the call is cancelled d…
DaveHanns a2ac5fd
fix: Search an oldText over 250 characters in time linear in the file
DaveHanns 6817b57
fix: Check create-actor file paths in time linear in the file count
DaveHanns 300b1d1
Merge remote-tracking branch 'origin/master' into feat/update-actor-v…
DaveHanns 0afdf69
fix: Cap update-actor-version at 20 edits per call
DaveHanns eced09b
fix: State facts instead of next calls in source tool texts
DaveHanns 43e34e4
fix: Mark create-actor and update-actor-version as open-world
DaveHanns 03a361a
fix: Take the create-actor version from .actor/actor.json
DaveHanns 873e03e
refactor: Start the post-write build with the client directly
DaveHanns 09812ec
refactor: Drop the post-write cancel checks in source write tools
DaveHanns f7445a7
test: Bound the 20-edit timing test at 6 s and say what it guards
DaveHanns a3d330a
fix: Read an empty actor.json version as none in create-actor
DaveHanns 3fd4b24
fix: State what the update-actor-version revision is for as a fact
DaveHanns cb07786
fix: Take a null expectedHash, expectedRevision, or versionNumber as …
DaveHanns 5c1d8ca
fix: Refuse a source path with a lone UTF-16 surrogate
DaveHanns be2dc8c
fix: Resolve a bare Actor name in the source tools
DaveHanns fb1c7c9
test: Read the only version for a null versionNumber in get-actor-ver…
DaveHanns 9dc70f8
Merge remote-tracking branch 'origin/master' into feat/update-actor-v…
DaveHanns 90f3108
test: Expect the taken-name error without the input hint master dropped
DaveHanns 0d5b6f6
refactor: Move the ID-or-name resolver out of the task helpers
DaveHanns 29ed1be
refactor: Import the Actor resolver from the shared resource ID module
DaveHanns 8092217
refactor!: Rename the source tool category to versions
DaveHanns 978d3bf
refactor: Move the versions tools to a versions folder
DaveHanns abfcc2b
fix: Start no build when update-actor-version changes nothing
DaveHanns 084952f
fix: Say only that no build was started when update-actor-version cha…
DaveHanns 7b6d9af
fix: Resolve a bare Actor name in build-actor
DaveHanns cf4e7de
fix: Resolve a bare Actor name in the build and run list filters
DaveHanns 29a14b9
fix: Refuse an update-actor-version edit that grows a file past 3 MiB
DaveHanns 12c6a0b
fix: Read the version of an .actor/actor.json that starts with a byte…
DaveHanns 81d0b5e
fix: Say a 5xx or unanswered version PUT may have saved the change
DaveHanns 68f10af
test: Check that update-actor-version refuses a NUL path in a single …
DaveHanns 0c7e960
test: Check the build hint of update-actor-version in a session witho…
DaveHanns ce891f4
test: Abort the build of the live create-actor case in finally
DaveHanns bc9e2c3
fix: Refuse a backslash in the path of a written file
DaveHanns 4984455
fix: Report a sub-resource reached by extra path segments in build-ac…
DaveHanns 49abeec
refactor: Extract redactEdits from redactSourceToolArgs
DaveHanns b2af38b
refactor: Split applyOperation of update-actor-version by operation type
DaveHanns e2527c0
refactor: Extract startBuildAfterWrite from respondAfterWrite
DaveHanns 61a58c4
refactor: Start the create-actor catch with a guard for a retried nam…
DaveHanns 0fc3c09
refactor: Check file and folder collisions with one path index in bot…
DaveHanns bd0daaf
fix: Say a version PUT that failed after a retry may have saved the c…
DaveHanns 766caf8
fix: Say no build was started after an unconfirmed save with autoBuild
DaveHanns 9ee727c
test: Check that update-actor-version refuses an oversized edit befor…
DaveHanns a2f8494
refactor: Write the byte order mark in create-actor as an escape
DaveHanns 4f5ff55
fix: Count only the LFs of newText that become CRLF in the length che…
DaveHanns e24a7e1
test: Log a failed build abort of the live create-actor case instead …
DaveHanns File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,208 @@ | ||
| import type { ActorCollectionCreateOptions, ActorVersionSourceFile } from 'apify-client'; | ||
| import { ActorSourceType, ApifyApiError } from 'apify-client'; | ||
| import dedent from 'dedent'; | ||
| import { z } from 'zod'; | ||
|
|
||
| import { APIFY_ERROR_TYPE_ACTOR_NAME_NOT_UNIQUE, HELPER_TOOLS } from '../../const.js'; | ||
| import { UserInputError } from '../../errors.js'; | ||
| import type { InternalToolArgs, ToolEntry, ToolInputSchema } from '../../types.js'; | ||
| import { TOOL_TYPE } from '../../types.js'; | ||
| import { compileSchema, fixZodSchemaRequired } from '../../utils/ajv.js'; | ||
| import { respondAborted, respondUserError } from '../../utils/mcp.js'; | ||
| import { createActorToolOutputSchema } from '../structured_output_schemas.js'; | ||
| import { buildFilesManifest, buildFilesRevision } from './source_files.js'; | ||
| import { | ||
| buildEmptyFilesWarnings, | ||
| buildSourceFileEntry, | ||
| extractSourceFiles, | ||
| redactSourceToolArgs, | ||
| resolveVersion, | ||
| respondAfterWrite, | ||
| respondToSourceToolError, | ||
| sourceFileArgs, | ||
| validateFileContent, | ||
| validateFilePath, | ||
| validatePathCharacters, | ||
| } from './source_helpers.js'; | ||
|
|
||
| const createActorArgs = z.object({ | ||
| name: z.string().min(1).describe('Name of the new Actor, without a username, for example my-scraper.'), | ||
| title: z.string().min(1).optional().describe('Display title, for example My Scraper.'), | ||
| description: z.string().optional().describe('A short description of what the Actor does.'), | ||
| files: z.array(sourceFileArgs).min(1).describe("The Actor's files, each with its whole content."), | ||
| versionNumber: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| 'Number of the version in MAJOR.MINOR form. Default: the version in .actor/actor.json, or 0.0 when it has none.', | ||
| ), | ||
| buildTag: z | ||
| .string() | ||
| .min(1) | ||
| .optional() | ||
| .describe('Tag that builds of the version get. The platform uses latest when it is omitted.'), | ||
| autoBuild: z | ||
| .boolean() | ||
| .default(false) | ||
| .describe('Start a build of the version after creating the Actor, and return without waiting. Default: false.'), | ||
| }); | ||
|
|
||
| /** The folders above a file, outermost first: `a` and `a/b` for `a/b/c.js`. */ | ||
| function listFolders(path: string): string[] { | ||
| const segments = path.split('/'); | ||
| return segments.slice(1).map((_, index) => segments.slice(0, index + 1).join('/')); | ||
| } | ||
|
|
||
| /** | ||
| * The entries to send, one per file; throws `UserInputError` for a path no file can be written at, and for a path | ||
| * given twice, since the platform puts the Actor name in the first .actor/actor.json and the build uses the last. | ||
| * Paths are checked through a set and a map, since comparing each file with every earlier one took 8 s for 20,000. | ||
| */ | ||
| function buildSourceFileEntries(files: readonly z.infer<typeof sourceFileArgs>[]): ActorVersionSourceFile[] { | ||
| const entries: ActorVersionSourceFile[] = []; | ||
| const filePaths = new Set<string>(); | ||
| const firstFileByFolder = new Map<string, string>(); | ||
| for (const [index, file] of files.entries()) { | ||
| validatePathCharacters(file.path, `files[${index}]`); | ||
| const entry = buildSourceFileEntry(file); | ||
| const label = `files[${index}] (${file.path})`; | ||
| validateFilePath(entry.name, label); | ||
| validateFileContent(entry, label); | ||
| if (filePaths.has(entry.name)) { | ||
| throw new UserInputError(`${label} repeats the path ${entry.name}; each path can be sent only once.`); | ||
| } | ||
| const folders = listFolders(entry.name); | ||
| const collision = folders.find((folder) => filePaths.has(folder)) ?? firstFileByFolder.get(entry.name); | ||
| if (collision !== undefined) { | ||
| throw new UserInputError( | ||
| `${label} collides with ${collision}; one path cannot be both a file and a folder.`, | ||
| ); | ||
| } | ||
| filePaths.add(entry.name); | ||
| for (const folder of folders) { | ||
| if (!firstFileByFolder.has(folder)) firstFileByFolder.set(folder, entry.name); | ||
| } | ||
| entries.push(entry); | ||
| } | ||
| return entries; | ||
| } | ||
|
|
||
| /** The `version` of .actor/actor.json when it is a non-empty string in valid JSON, as `apify push` reads it. */ | ||
| function readActorJsonVersion(entries: readonly ActorVersionSourceFile[]): string | undefined { | ||
| const actorJson = entries.find(({ name, format }) => name === '.actor/actor.json' && format === 'TEXT'); | ||
| if (!actorJson) return undefined; | ||
| try { | ||
| const { version } = JSON.parse(actorJson.content) as { version?: unknown }; | ||
| return typeof version === 'string' && version !== '' ? version : undefined; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * https://docs.apify.com/api/v2/actors-post | ||
| * /v2/actors | ||
| * | ||
| * One POST creates the Actor and its version together, private and in the token's account. apify-client retries the | ||
| * POST after a network error, a timeout, a 429, or a 5xx, so when the platform saved an attempt it did not answer, the | ||
| * retry fails on the name that attempt took, and the call says the Actor may exist. | ||
| */ | ||
| export const createActor: ToolEntry = Object.freeze({ | ||
| type: TOOL_TYPE.INTERNAL, | ||
| name: HELPER_TOOLS.ACTOR_CREATE, | ||
| title: 'Create Actor', | ||
| description: dedent` | ||
| Create a new private Actor in your account, with one version holding the files you give. It never changes an existing Actor: the platform refuses a name your account already uses. | ||
| - files: every file with its path and content. Binary files take base64 content with encoding base64; files with a binary extension such as .png default to it. | ||
| The platform sets the name field of .actor/actor.json to the Actor name, so the result lists each file's hash and the version's revision as stored. | ||
| autoBuild starts a build after creating the Actor and returns without waiting. | ||
|
|
||
| USAGE: | ||
| - Use to upload new Actor code to the Apify platform for the first time. | ||
|
|
||
| USAGE EXAMPLES: | ||
| - user_input: Create an Actor called hacker-news-scraper from these files`, | ||
| // `fixZodSchemaRequired` strips `autoBuild` from `required` because it has a default. | ||
| inputSchema: fixZodSchemaRequired(z.toJSONSchema(createActorArgs)) as ToolInputSchema, | ||
| outputSchema: createActorToolOutputSchema, | ||
| ajvValidate: compileSchema(z.toJSONSchema(createActorArgs)), | ||
| annotations: { | ||
| title: 'Create Actor', | ||
| readOnlyHint: false, | ||
| destructiveHint: false, | ||
| // A second call with the same name is refused rather than repeated. | ||
| idempotentHint: false, | ||
| // autoBuild starts a build, like build-actor. | ||
| openWorldHint: true, | ||
| }, | ||
| redactArgs: (args: Record<string, unknown>) => | ||
| redactSourceToolArgs(args, { | ||
| keys: ['name', 'title', 'description', 'versionNumber', 'buildTag', 'autoBuild'], | ||
| listKey: 'files', | ||
| entryKeys: ['path', 'encoding'], | ||
| }), | ||
| call: async (toolArgs: InternalToolArgs) => { | ||
| const { args, apifyClient: client, signal, loadedToolNames } = toolArgs; | ||
| const parsed = createActorArgs.parse(args); | ||
| // A cancel before the POST creates nothing; per the MCP spec the cancelled request gets no response. | ||
| if (signal?.aborted) return respondAborted(); | ||
| try { | ||
| const sourceFiles = buildSourceFileEntries(parsed.files); | ||
| // The platform refuses a version without a number, so this defaults to what `apify push` uses. | ||
| // AJV turns a null versionNumber into an empty string, which counts as not given. | ||
| const versionNumber = parsed.versionNumber || readActorJsonVersion(sourceFiles) || '0.0'; | ||
| const created = await client.actors().create({ | ||
| name: parsed.name, | ||
| ...(parsed.title !== undefined && { title: parsed.title }), | ||
| ...(parsed.description !== undefined && { description: parsed.description }), | ||
| versions: [ | ||
| { | ||
| versionNumber, | ||
| ...(parsed.buildTag !== undefined && { buildTag: parsed.buildTag }), | ||
| sourceType: ActorSourceType.SourceFiles, | ||
| sourceFiles, | ||
| }, | ||
| ], | ||
| } satisfies ActorCollectionCreateOptions); | ||
| const fullName = `${created.username}/${created.name}`; | ||
| // The platform sets the name in .actor/actor.json on create, so the stored files, not the sent ones, match | ||
| // a later read. | ||
| const version = resolveVersion(created, undefined, fullName); | ||
| const files = buildFilesManifest( | ||
| extractSourceFiles(version, `Version ${version.versionNumber} of ${fullName}`), | ||
| ); | ||
| const emptyPaths = files.filter(({ sizeBytes }) => sizeBytes === 0).map(({ path }) => path); | ||
| return await respondAfterWrite({ | ||
| toolArgs, | ||
| autoBuild: parsed.autoBuild, | ||
| target: { actorId: created.id, versionNumber: version.versionNumber }, | ||
| structuredContent: { | ||
| actorId: created.id, | ||
| fullName, | ||
| versionNumber: version.versionNumber, | ||
| revision: buildFilesRevision(files), | ||
| files: files.map(({ path, sizeBytes, hash }) => ({ path, sizeBytes, hash })), | ||
| ...buildEmptyFilesWarnings(emptyPaths), | ||
| }, | ||
| summary: `Created the private Actor ${fullName}.`, | ||
| }); | ||
| } catch (error) { | ||
| if ( | ||
| error instanceof ApifyApiError && | ||
| error.type === APIFY_ERROR_TYPE_ACTOR_NAME_NOT_UNIQUE && | ||
| error.attempt > 1 | ||
| ) { | ||
| const readTool = loadedToolNames.includes(HELPER_TOOLS.ACTOR_VERSION_GET) | ||
| ? `; ${HELPER_TOOLS.ACTOR_VERSION_GET} reads its files` | ||
| : ''; | ||
| return respondUserError( | ||
| `The platform refused the name ${parsed.name} as taken when the request was retried, so an earlier ` + | ||
| `attempt of this call may have created the Actor ${parsed.name}. ` + | ||
| `If it holds the files you sent, this call created it${readTool}.`, | ||
| { httpStatus: error.statusCode }, | ||
| ); | ||
| } | ||
| return respondToSourceToolError(error); | ||
| } | ||
| }, | ||
| } as const); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
source is a poor category name. It's a term we made up: neither the Apify API nor the Apify CLI uses it. The API calls this "Actor versions", and the CLI has no matching group (Actor code goes through actors push and actors pull).
I'd rename it to versions. That's not a great name either, because createActor isn't strictly a versions endpoint. But it matches the API vocabulary, and it stays consistent with runs, builds and tasks.
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.
Agree.
versionssounds better.