Skip to content

feat!: Add create-actor and update-actor-version tools - #1453

Open
DaveHanns wants to merge 121 commits into
masterfrom
feat/update-actor-version
Open

DaveHanns wants to merge 121 commits into
masterfrom
feat/update-actor-version

Conversation

@DaveHanns

@DaveHanns DaveHanns commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

What

Two write tools in the versions category, which this PR renames from source:

  • create-actor creates a private Actor with its first version and files. It never touches an existing Actor.
  • update-actor-version writes, edits, or deletes files of a version in one call. Files the call does not mention stay as they are.

Why

They replace the drafted push-actor (#1333), following the Actor source tools spec (#1452). Closes #1463. An edit sends only the changed text, not whole files.

How

  • create-actor: name, title, description, files, versionNumber, buildTag, and autoBuild. One POST creates the Actor and its version. It returns the files as the platform stored them, since the platform sets the name in .actor/actor.json.
    • Without versionNumber, the version is the version in .actor/actor.json when that is a non-empty string, and 0.0 otherwise, the order apify push uses. A version that is not MAJOR.MINOR, such as 1.0.0, goes to the API as is, and the API refuses it.
  • update-actor-version: actor, versionNumber, operations, expectedRevision, and autoBuild.
    • write creates a file, or replaces one given its expectedHash.
    • edit replaces text; each oldText must match exactly once, overlapping matches included, and an expectedHash, when given, must match the file. In a file with CRLF line breaks, an LF edit is applied as CRLF, so the file keeps one line ending.
    • delete removes a file given its expectedHash. A rename is a delete and a write.
  • The actor argument (update-actor-version, and get-actor-version on master) takes an ID, a full name (username/name or username~name), or a bare name, which resolves to the Actor of that name in your account. fetchActor uses getResourceByIdOrName, which tasks and schedules already use: a bare name that cannot be an ID goes out once as ~name, and an ID-shaped value is tried as an ID first, then as ~name. That helper and its two siblings move from task_helpers.ts to src/utils/apify_resource_id.ts, since they resolve Actors, tasks, and schedules alike. The move is its own commit with no behavior change. build-actor, get-actor-build-list, and get-actor-run-list resolve a bare name the same way, so a name one call accepts works in the next (Resolve bare Actor names in build-actor, call-actor, and the other Actor tools #1614).
  • The operations apply to a fresh read of the version, all or nothing. A failed check names the operation, the file, and a reason code: FILE_EXISTS, FILE_NOT_FOUND, HASH_MISMATCH, NO_MATCH, MULTIPLE_MATCHES, NOT_TEXT, or REVISION_MISMATCH.
  • One PUT per call carries only sourceType and sourceFiles, never env vars or the build tag. A call that changes nothing sends no PUT and starts no build, even with autoBuild; it says "No build was started."
  • Limits: update-actor-version takes at most 100 operations and at most 20 edits in total across them; a larger call is refused before the version is read. Each edit searches the whole file text, which can be close to 3 MiB, and blocks the event loop while it runs: up to about 20 ms for an oldText over 250 characters, so 20 edits take up to about 0.4 s. Writes and deletes never search a stored file. An edit is refused before its search once it could make the file longer than 3 MiB (in UTF-16 code units), so edits that grow the text cannot stretch that bound. An oldText over 250 characters is searched with Knuth-Morris-Pratt, since V8's indexOf keeps its Boyer-Moore tables for only the last 250 characters of a pattern and slows down to the file length times the pattern length past them. create-actor checks its paths with a set and a map instead of comparing each file with every earlier one, which took 8 s for 20,000 files.
  • Revision: the first 16 hex characters of the SHA-256 of the JSON array of [path, hash] pairs, sorted by path. JSON quotes and escapes each path, so two different file sets never give the same revision. get-actor-version on master uses the same function, so its revision values change once with this PR.
  • Both tools: encoding defaults to base64 for binary extensions such as .png. warnings names empty files, which the build skips. autoBuild (default false) starts a build without waiting for it; a build that cannot start is reported in buildError, and the write stands. Both tools have openWorldHint: true, since autoBuild starts a build, as build-actor does.
  • null inputs: the input check (AJV with coerceTypes) turns a null into an empty string before the tool runs. An empty expectedHash, expectedRevision, or versionNumber counts as not given, since no stored hash, revision, or version number is empty. An empty content stays an empty file: the tool cannot tell it from a null, and warnings names empty files.
  • Cancellation: a cancel that arrives after the write, including during the Console link lookup before the build start, gets the cancelled-call response. The write stays saved and no build starts. The build starts without the request's signal, so a build that started is never aborted.
  • Logged arguments: both tools set redactArgs. The logged copy is an allowlist: it keeps the Actor, version, paths, operation types, encodings, and hashes, and replaces file content and the oldText and newText of edits with [REDACTED]. File content can hold secrets, such as an API key written into the code.
  • Refused before anything is written: a path with a control character (U+0000 to U+001F or U+007F) or a lone UTF-16 surrogate, a written path with a backslash, a path outside the Actor root (a leading / or ..) or at a folder path, a new file whose path collides with a file or folder, the same path twice in create-actor, base64 that does not decode without loss, and text with a lone UTF-16 surrogate. The platform accepts all of these, and the build then fails or stores a corrupt file. UTF-8 stores a lone surrogate as U+FFFD, so the stored name would no longer match the revision.
  • Served text follows TOOL_GUIDELINES.md: descriptions, refusals, failed checks, and nextStep state facts and never ask for another call, for example "No build was started; runs use these files only after the version is built."
  • A save that may have landed: when the version PUT fails with a 5xx, without an HTTP response, or after apify-client retried it, the reply says the platform may still have saved the change and names the revision each outcome leaves.
  • A retried create: apify-client can retry the create-actor POST, and the platform then refuses the name as taken. The tool says an earlier attempt of the same call may have created the Actor, and that if it holds the files sent, this call created it.
  • Left to the platform: permissions, Actor names, the 3 MiB limit, path length, and version numbers. Its error reaches the agent unchanged. apify-api-write (feat: Add apify-api-write to call any Apify API write method #1445, on master) covers env vars, build tags, Git sources, and replacing every file at once.
  • The version lookup, the refusal of versions not stored as files, and the URL credential hiding move from get_actor_version.ts to source_helpers.ts, so all source tools share them.

Testing

Unit tests cover each operation and reason code, all or nothing, the PUT body, CRLF edits, encodings, warnings, expectedRevision, refusals, API errors, cancellation, autoBuild, and schema conformance. Round-trip tests check that the revision these tools return is the one get-actor-version then reads.

Regression tests for the reviews:

  • 100 operations and 20 edits pass; 101 operations or 21 edits are refused before the version is read.
  • 20 edits with an oldText over 250 characters on a file of almost 3 MiB finish within 6 s. They take about 0.5 s locally, and CI runs this file about 4.5 times slower.
  • A 10,001-character oldText against a file of almost 3 MiB returns NO_MATCH within a second (it took over 4 s), and oldText over 250 characters still finds overlapping matches and single matches after partial ones.
  • create-actor checks 20,000 files within a second (it took about 8 s), and file and folder collisions two levels deep keep their messages.
  • The logged arguments of both tools hold no trace of a synthetic secret placed in file content, oldText, newText, and unknown keys, and the arguments the tool receives are unchanged.
  • A path with NUL, a line break, a tab, U+001F, U+007F, or a lone UTF-16 surrogate is refused in every file and operation, before any request; the delete, delete, and write call from the first review is refused this way. A stored name with NUL and a line break, or with a quote and a comma, no longer gives the revision of a different file set.
  • A cancel during the Console link lookup leaves the write saved and starts no build.
  • A null expectedHash, expectedRevision, or versionNumber passes the input check and counts as not given, in update-actor-version, get-actor-version, and create-actor. A write over an existing file with a null expectedHash still gets FILE_EXISTS.
  • A bare name goes out once as ~name; a 17-character value is tried as an ID, then as ~name; username/name, username~name, and ~name are looked up once.
  • create-actor takes the version from .actor/actor.json, sends 0.0 for no file, no version, an empty or non-string version, or a file that is not JSON, and sends versionNumber when given.
  • The update-actor-version description lists the reason codes and asks for no retry.

type-check, lint, format, test:unit, and check:agents pass.

Five live integration cases in tests/test_kit/cases/versions.cases.ts run in CI against the platform:

  • create-actor, then get-actor-version reading back the same hashes and revision, and build-actor starting a build, both given the bare name
  • one update-actor-version call that creates, replaces, edits, and deletes files and leaves the other files, env vars, and build tag as they were
  • a failed write, edit, and revision, each with its reason code and nothing written
  • a version stored in a Git repository, refused with its credentials hidden
  • a name the account already uses

Each case uses its own test-source-* Actor and deletes it at the end. Only the create-actor case starts a build, and it aborts it right away.

Notes

  • get-actor-version (feat: Add get-actor-version tool #1450) is on master. create-actor-version and delete-actor-version (feat: Add create-actor-version and delete-actor-version tools #1454) stack on this PR.
  • Contract change for get-actor-version, which is on master: its actor text and not-found text change, a bare name now resolves, and a null versionNumber counts as not given. The second review round also changed the served text and openWorldHint of the two new tools.
  • Contract change for build-actor, get-actor-build-list, and get-actor-run-list, also on master: a bare Actor name now resolves to one of your own Actors, and their actor or actorId text and not-found text say so.
  • Breaking: the source category is renamed to versions. source shipped in v0.17.2 with only get-actor-version. tools=source is now a retired selector, like get-actor-log: it loads no tools, and the README says to use tools=versions. The tools moved from src/tools/source/ to src/tools/versions/ as pure renames.
  • apify-mcp-server-internal keys CATEGORY_META on source (src/web/src/constants.ts), so the package bump that brings this rename needs that key renamed to versions there.
  • Merge feat: Resolve a bare Actor name in call-actor and fetch-actor-details #1615 into this branch first. It makes call-actor and fetch-actor-details resolve a bare name too. In the evals, agents passed a bare name to call-actor 8 times once the versions tools and build-actor accepted it; with this PR alone, those calls fail.
  • A build-actor call sent in the same turn as an update-actor-version write can start before the write lands and build the old files. autoBuild avoids this. In the evals it happened once, and the platform started that build after the write.
  • Follow-ups outside this PR: delete-actor still refuses a bare name, and the Specify versionNumber; this Actor has versions: ... text, which build-actor shares, still asks for a value. Both are better changed on master for all tools at once.
  • TODO comments mark where update-actor-version reads and writes a whole version, since the Apify API has no per-file operations.
  • There is no lock between concurrent calls. A save that lands between the read and the PUT is not detected until the platform has a conditional version PUT.
  • Follow-up for the read-to-PUT race: Concurrent update-actor-version calls can overwrite each other's changes #1523

AI disclosure: implemented with Claude Code; awaiting human review.

🤖 Generated with Claude Code

Adds get-actor-version, the read side of the source tools that replace
push-actor and pull-actor, in a new source category that is not enabled
by default.

It returns a version's metadata, a file manifest (path, decoded size,
hash, format), a revision for the whole file set, and the content the
caller asks for within MAX_INLINE_BYTES. Without paths it returns every
text file only when all of them fit, so one large file cannot crowd out
the rest; paths: [] gives the listing only; named paths fill the limit
in order, and a single large text file can be read in line ranges.
Environment variables come back as names and isSecret only.

The hash is the first 16 hex characters of the SHA-256 of the file's
bytes, and the revision hashes the sorted path and hash lines, so both
are the same for a version stored inline and one stored as a zip.

Zip-stored versions are read only from a key-value store record on the
session's own API host, with the caller's token. The zip is parsed from
its central directory with no new dependency, because apify push writes
zeros for sizes in local headers. Archives over 50 MiB, over 10,000
entries, or over 64 MiB uncompressed are refused, as are encrypted,
zip64, symlink, and unsafe or duplicate names, and every entry's length
and CRC-32 are checked after inflating. Git, gist, and outside zip
sources return only their URL, without its query string.
Zip reading:
- Refuse an archive whose end record declares fewer entries than the
  central directory holds, and an entry whose local header names another
  file or method, so the files reported match what other zip readers see.
- Check entry names for length, NUL, absolute paths, and '..' first, so
  no refusal message repeats an overlong name.
- Skip a root folder entry such as './' instead of refusing the archive.
- Detect text with isUtf8 and decode only the files that are returned,
  so a listing holds only the inflated bytes the 64 MiB cap bounds.

Inline files:
- Read a file stored without format as TEXT and one without content as
  empty, as the build worker does, instead of failing or breaking the
  output schema.
- Normalize inline paths the way the zip reader does, so the same files
  give the same manifest and revision however they are stored.
- Return a UTF-8 file stored as BASE64 as text, as a zip read does. The
  manifest keeps the stored format.

Results:
- A line over the 256 KiB limit no longer fails the call. The file goes
  to omittedPaths and the summary names the startLine that skips it, or
  says a single-line file cannot be read by lines.
- The summary no longer tells the caller to name a base64 file too large
  to return; it points to the Apify CLI instead.
- Remove the query string from Git and gist URLs too, and http user and
  password from every URL, before returning them or hashing the revision.
- Word the outside zip summary without claiming the source is not on
  Apify.
- The description says the API hides the source of most Actors the
  caller cannot modify, and gives the macOS hash command.

Tests cover each rule and the boundaries that no test pinned before:
hidden sources for URL types, a record URL on another host, local extra
fields that differ from central ones, byte order of the manifest, base64
files next to text at the limit, and the entry count, size, CRC, and
encryption flag edges.
Add two write tools to the opt-in source category. Both write only to the
caller's own account and store files inline in the version.

create-actor creates a private Actor with one version in a single POST, from
files (which must include .actor/actor.json) or a Git repository. It refuses a
name the account already uses, and warns about a missing Dockerfile and empty
files.

update-actor-version applies write, edit, delete, and move operations to a
fresh read of the version and saves the result with one version PUT that
carries only the source keys and buildTag, never envVars. Each operation has a
precondition (an absent path, expectedHash, or an exact-once match), and
expectedRevision guards replaceFiles and gitRepoUrl. A failure writes nothing
and names the operation, the path, and a reason code. It refuses zip-stored
versions and results over the platform's 3 MiB limit, and refuses file
operations on Git and gist versions. A save that lands between the read and
the PUT is not detected until the platform gets a conditional version PUT.

Both take autoBuild, which starts a build after the write and returns without
waiting. get-actor-version's description now says its hashes and revision are
what update-actor-version takes, when that tool is loaded.
- Check for a cancel right before the version PUT and the Actor POST, so a
  request cancelled during the reads writes nothing.
- create-actor builds its files, hashes, and revision from the files the
  platform stored, since it sets the name field of .actor/actor.json on
  create, and warns when that changed the file. A taken name (including the
  race case, name-not-unique) now gives the full name update-actor-version
  takes. The missing Dockerfile warning says the build uses the platform's
  default Node.js Dockerfile, and matches paths regardless of case.
- update-actor-version refuses a missing .actor/actor.json only when the
  changes remove it, or when a Git version switches to stored files.
- Sending back the Git URL that get-actor-version shows no longer removes
  the stored credentials. Another URL on the same host keeps them, with a
  warning, and a URL on another host is refused.
- Stored entries the call leaves alone go back verbatim and in place,
  duplicates included; a changed file keeps its stored name.
- Refuse a result where a file path is also a folder of other files or a
  folder entry.
- Cap the NO_MATCH context at 4 KiB, show a window of a long line, show the
  lines around newText when only it is found, and suggest reading again when
  no context is shown.
- Share splitLines, formatMib, the MiB and 255-character constants, and the
  500-file cap across the source tools, and give the 2 MiB refusal text that
  fits each tool.
- Output schema texts no longer name another tool, and describe sourceType
  and changes as the tool returns them.
- Add tests for the new rules and for the cases the reviewers found untested.
Move the files input rules, the manifest builder, and the missing Dockerfile
warning from create-actor to source_helpers.ts, so create-actor-version can
use the same rules. create-actor behaves as before.

(cherry picked from commit ea8fbb4)
The Apify API cannot read or change single files of an Actor's source, so the version GET returns every file and a zip-stored version is downloaded and unpacked whole, even for one file. TODO comments mark both places for when such an API exists.
The Apify API cannot change single files of an Actor's source, so a one-line edit reads the whole version and writes all its files back, which also leaves the gap between the read and the PUT. A TODO marks the place for when an API for atomic source changes exists.
…y it

Remove the zip reader, its test helper, and its tests. A version stored as a
zip in a key-value store of this API is now refused as a user error; a zip at
an outside URL is still reported by its URL. A TODO marks where zip reading
comes back and the library planned for it.

Other simplifications, with the same output data:
- One summary function replaces the format helpers.
- A line over the limit is reported only when nothing of the range fits.
- No Apify CLI hint for files too large to return.
- Paths sort in plain JavaScript string order instead of UTF-8 byte order,
  for the manifest and the revision lines.
- One hash helper, and the manifest builder and folder check live in
  source_files.ts.
# Conflicts:
#	src/tools/AGENTS.md
#	src/tools/source/get_actor_version.ts
#	src/tools/source/source_archive.ts
#	src/tools/source/source_files.ts
After the merge of get-actor-version's simplification:
- create-actor and update-actor-version use buildFilesManifest and
  isFolderEntry from source_files.ts, and source_helpers.ts drops its copies.
- The absolute path check (ABSOLUTE_NAME_REGEX) now lives in source_files.ts,
  since the zip reader it came from is gone.
- update-actor-version refuses a zip-stored version with the same wording as
  get-actor-version and no longer points to apify push.
- Say that the tool does not download zips when it reports an outside
  zip URL, since a URL on the session's own API host is not outside the
  Apify API.
- Describe the path normalization correctly: empty and `.` segments are
  dropped as the build worker does, and backslashes become `/`, which
  the build worker does not do for inline names.
- Pass only the API base URL to the version reader, since it no longer
  calls the API, and fix comments that still mention the source store.
- Stop exporting the hash helper and the format type, and read an
  entry's name, format, and content in one destructuring.
- Test that a BASE64 file with a text extension and invalid UTF-8 comes
  back as base64, that a byte order mark is kept, and that the last of
  two entries with the same path wins.
# Conflicts:
#	src/tools/source/source_files.ts
Move the rule that turns a stored entry into bytes into source_files.ts
as decodeSourceFileEntry, and use it for the hash that get-actor-version
reports, for the unchanged-file check in update-actor-version, and for
the config comparison in create-actor. The three can no longer disagree
about how a missing format or content is read. A null content now reads
as empty in all three, as it already did for the hash.
Narrow the tool to what the spec asks for:
- Without paths, return the file list only. Drop pathPrefix and the
  read-all-text-files default.
- Keep basic line ranges; a range over the limit goes to omittedPaths.
  Drop the automatic range and the long-line handling.
- Refuse every version not stored as files with one message that names
  its URL without credentials. Drop Git, gist, and zip reporting, the URL
  revision, and the split of Git URLs.
- Read the version from the Actor GET, which already hides the source the
  same way. Drop the second GET, the 4xx catch, and the no-versions branch.
- Drop env var names, the stored format, the source type, and the build
  tag from the output, and keep the text result to one sentence.
- Normalize stored names with posix.normalize, as the build worker does.
Hide the password of a URL in any scheme, not only http and https, so an
ssh:// Git URL with a password no longer shows it in the refusal. The SSH
user stays.

Test that a BASE64 file with a binary extension comes back as base64 even
when its bytes are valid UTF-8.
# Conflicts:
#	README.md
#	src/tools/AGENTS.md
#	src/tools/source/get_actor_version.ts
#	src/tools/source/source_files.ts
…rsion simplification

The simplified get-actor-version dropped helpers that create-actor and update-actor-version still import. Define them in source_helpers.ts until the write tools are simplified too.
Narrow both tools to the updated spec and leave to the platform the checks it already makes.

- update-actor-version takes write, edit, and delete operations, reads the version from the one Actor GET, and saves it with one PUT of sourceType and sourceFiles, skipped when the revision does not change. Move, replaceFiles, gitRepoUrl, buildTag, allOccurrences, edit excerpts, match context, caps, and the size and path checks are gone. The edit logic moves in from source_edits.ts, which is deleted.
- create-actor takes files only and sends versionNumber and buildTag only when given. The name rules, token check, defaults, caps, path and base64 checks, actor.json rules, Git source, and extra warnings are gone.
- The Actor lookup no longer compares the owner with users/me, and API errors reach the agent unchanged.
- The Actor lookup, version choice, and refusal of versions not stored as files move to source_helpers.ts, shared with get-actor-version. The build start after a write and its next step are written once.
- Results are shorter: update returns revision, changed, and changes with each file's new hash; both return build or buildError and the empty-files warning.
… simplification

create-actor sends version number 0.0 again when the caller gives none. The platform refuses a version without a number, so the default call failed and created nothing. The build tag is still sent only when given, since the platform sets latest on the only version.

The create-actor test mock no longer fills in a version number the platform does not add, and the update-actor-version test comment now describes the PUT file order the code produces.
# Conflicts:
#	tests/unit/tools.mode_contract.test.ts
# Conflicts:
#	tests/unit/tools.mode_contract.test.ts
Any other error from starting the build is a bug, so it goes to the tool-call engine
as a server error instead of reading as a failed start.
# Conflicts:
#	README.md
#	src/tools/registry.ts
# Conflicts:
#	README.md
#	src/tools/registry.ts
The tools folders match the category names, like builds, runs, and
tasks. Only import paths and the folder name in src/tools/AGENTS.md
change.
With autoBuild and nothing to write, the call now starts no build and says
"Nothing changed, so no build was started." Without autoBuild, it says only
"No build was started.", since the stored files may already be in the build
runs use. create-actor always writes and keeps its text.
@DaveHanns DaveHanns changed the title feat: Add create-actor and update-actor-version tools feat!: Add create-actor and update-actor-version tools Oct 16, 2026
@DaveHanns
DaveHanns requested a review from MQ37 October 16, 2026 14:23
build-actor now looks the Actor up with getResourceByIdOrName, like the
versions tools: a name that is not ID-shaped goes out once as ~name, and
an ID-shaped value is tried as an ID first, then as ~name. The actor
description and the not-found text say a name alone works for your own
Actors. The live bare-name case also builds by the bare name.
The actorId filter of get-actor-build-list and get-actor-run-list now
goes through getResourceByIdOrName. The list endpoints take ~name, so a
name costs no extra request; only an ID-shaped value that matches no ID
is listed a second time, as ~name. The actorId descriptions and the
not-found text say a name alone works for your own Actors.
@DaveHanns

Copy link
Copy Markdown
Contributor Author

Agent evals for the version tools

Five cases cover the three flows from #1452. The agent gets only the MCP tools. A trial passes when the judge passes it and no tool call fails. Platform check: the final build started after the last write and succeeded, the final run used it and returned the expected item, and only the intended files changed.

Server: a local merge of #1454 at 8d51cb99 (it includes #1453 at cf4e7deb) and #1615 (bare Actor names in call-actor and fetch-actor-details) at 74b20dd7. That PR's later commits up to c49a92de (one error reply, one phrase of tool text) were not evaluated. Opus 4.8 stands in for Opus 5.5, which needs a newer Claude Code than the eval harness pins.

Model Judge No tool errors Checked on the platform
Haiku 5.5 5/5 5/5 5/5
Sonnet 5.5 5/5 5/5 5/5
Opus 4.8 5/5 4/5 5/5

Previous round (same cases, without the bare-name changes): judge 15/15, no tool errors 7/15, 8 tool errors: 7 Actor names without a username in build-actor and fetch-actor-details, 1 Haiku waitSecs over 45. This round, all 20 such names succeeded: 11 in build-actor (from #1453), and 8 in call-actor and 1 in fetch-actor-details (from #1615).

Remaining error: Opus 4.8, create-python-scratch. After create-actor warned that the build skips the empty src/__init__.py, the agent wrote to it without expectedHash. update-actor-version refused with FILE_EXISTS and named the hash, and the retry succeeded. The warning could say that the write needs the hash.

Not an error: in Sonnet 5.5's multi-file-rename-module, build-actor overlapped the update-actor-version write in the same turn. The platform started that build after the write ended, and it failed with a Docker "context canceled" at COPY. The rebuild, 0.0.3, succeeded and was checked to start after the last write.

Cleanup: all Actors from the runs were deleted, and none are left.

The cases are in the staging dataset mcp-server-evals-merge-versions-staging. Moving them into mcp-server-evals-merge is a follow-up.

🤖 Generated with Claude Code

@MQ37 MQ37 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for item 1: the documented event-loop bound doesn't hold once edits grow the text, and the fix is small. The rest is feedback so far, and I still need more time to test and play with this. It looks like it works otherwise.

Re-checked at cf4e7de in rootless Podman against a mocked Apify API (real stdio server, stateful loopback mock, no network), plus mutations of the source against the unit tests, and 40 runs of Haiku and Sonnet driving the tools. Every point from my two reviews is fixed. Remaining:

  1. Edit growth blocks the event loop (required). The 20-edit cap assumes a stored file of at most 3 MiB, but edits can grow the text within one call, each later edit searches the grown text, and the platform checks the size only at the PUT, after all the CPU work. With a 9 MiB body (dev_server; stdio caps at 10 MiB), 20 operations on a base64 CRLF file block the event loop for 1.4-1.7 s, against 0.14-0.28 s for the documented case. Fix, tested in a patched copy: after each edit, refuse with a UserInputError once the text passes 3 * 1024 * 1024 (about 4 lines, plus a test; both caps stay). The same requests then block 20-43 ms, and 148-183 ms for the base64 CRLF case because the CRLF conversion of the edit text runs before the check, so the check should sit before it. The 242 update tests still pass. A 100 KB request still blocks about 0.3-0.4 s, which is the documented bound. Separate question, not blocking: tool calls have no concurrency limit, so one token with 10 concurrent sessions of the worst 100 KB request (measured on dev_server; the hosted express.json() default cap is 100 KB per the code) keeps a pod 92% blocked with 2.5-3.3 s stalls. Should that live in the server layer?
  2. BOM in .actor/actor.json makes the version 0.0 silently: the create succeeds and the agent blames the tool default. Agents keep the BOM when the user says to keep files exactly as they are (5/5 runs; 0/2 with neutral wording). Strip a leading U+FEFF before JSON.parse, or warn when actor.json does not parse (1 line).
  3. Error after a saved write. When the PUT fails with a 5xx after the platform stored it, the reply says "usually temporary", and 4 of 8 agents retried blindly. The expectedRevision/expectedHash guards stopped every retry, but an edit sent with neither (1 of 8) would apply twice. Could the 5xx error of update-actor-version say the change may have been saved and to read the version first, like create-actor's "may have created"?
  4. Tests. Skipping the path-character check for delete operations fails no test, and for write only the revision-collision test fails. The "in any operation" cases all assert operations[1], which is the edit; one it.each over write/edit/delete with a single NUL path fixes it (about 12 lines). The nextStep at source_helpers.ts:331 is tested only with every tool loaded. In the first live case the build abort is not in finally, so a failed assertion leaves a started build running. About 240 test lines repeat shared-helper cases across the tools.
  5. tools=source. The configurator in apify-mcp-server-internal (develop, url_utils.ts:38) writes a fully selected category as its id, and source held only get-actor-version, so existing links now load nothing. Is that intended? If not, map source to get-actor-version only, not to versions, which would add the write tools.

Nits:

  • src\main.js and .\Dockerfile are stored as literal names without a warning. No agent sent them in 15 runs, but refusing \ next to the control characters is 3 lines.
  • build-actor with john/my-actor/runs/last crashes with a TypeError in listVersionNumbers (build_helpers.ts:18); fetchActor has the guard. This predates the PR.

Code quality:

  • redactSourceToolArgs (source_helpers.ts:64-70): the edits branch nests five levels (spread, ternary, map, arrow, ternary). Extract redactEdits.
  • applyOperation (update_actor_version.ts:225-272, 47 lines) falls through to the edit branch after the write and delete branches return. Split it into applyWrite, applyEdit and applyDelete. The update call is 67 lines; applyOperations can come out of it.
  • respondAfterWrite (source_helpers.ts:290-333) declares let linkContext and let build outside the try. Extract startBuildAfterWrite() returning { build } | { buildError }.
  • The create_actor.ts:189-205 catch can start with a guard: if (!isRetriedNameTaken(error)) return respondToSourceToolError(error);.
  • The file/folder collision rule is written twice: a Set/Map in create-actor, a linear scan in validateNewFilePath. One helper would do.

The edit cap assumed a file of at most 3 MiB, but edits could grow the text within one call, each later edit searched the grown text, and the platform checks the size only at the PUT. A 9 MiB request blocked the event loop for over a second.

Each edit now fails before its CRLF conversion and its search when the file could pass 3 MiB of UTF-16 code units. In a CRLF file each character of newText counts as two, so the text never passes the limit.
The edits branch of an entry's logged copy nested five levels deep. The logged copy is unchanged.
applyOperation fell through to the edit branch after the write and delete branches returned. Each type now has its own function, and applyOperations takes the loop out of the tool call. Checks, their order, and the errors are unchanged.
respondAfterWrite declared the link context and the build outside the try that set them. The build start now returns the build, the error, or the cancel, and respondAfterWrite builds the response from it. The responses are unchanged.
…e refusal

The catch nested its one special case in an if. isRetriedNameTaken now names the case, and every other error returns first. The responses are unchanged.
…h write tools

create-actor checked them with a set and a map, and update-actor-version with a scan of every file per write. SourcePathIndex now holds the rule for both: it looks up only the folders of a path, and update-actor-version adds and deletes paths in it as its operations run. Each refusal names the same collision as before.

listFolders now finds the slashes with indexOf, with the same output, so indexing 20,000 files takes about 6 ms instead of 13 ms.
…hange

apify-client retries a PUT after a 429, a 5xx, or a network error. When an earlier attempt was saved but not answered and the last one got a 4xx such as a 429, the error was rethrown, and a retried call without hash or revision checks would apply its edits twice. An ApifyApiError from an attempt after the first now gets the reply that names the revisions, whatever its status.
The reply to a version PUT that may have saved the change did not say what happened to the requested build. None starts, so runs keep the previous build even when the save went through. With autoBuild, the reply now ends with "No build was started."
…e searching for its oldText

Only the check before the next edit's search was pinned, so moving the length check after the search of the same edit passed every test. The new case has an oldText that is not in the file and a newText that passes the limit, and expects the length refusal, not NO_MATCH.
The regex that skips a leading byte order mark in .actor/actor.json, and the test of it, held the character itself, which no editor shows. Both now use the \uFEFF escape. The behavior is unchanged.
…ck of an edit

In a file with CRLF line breaks, the check counted every character of newText twice, so an edit whose result fits could be refused, for example a newText of 0.4 MiB without line breaks in a file of 2.5 MiB. It now adds one code unit per LF without a CR before it, the LFs the conversion turns into CRLF, still before the conversion and the search. The refusal no longer needs its CRLF sentence.

Counting reads newText once: about 50 ms for the 4.5 million LFs a 9 MiB request body can hold.
…of throwing it

The abort in finally ran in a try whose error replaced the error of a failed check. It is now caught and logged, and the Actor is still deleted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-builders Issues owned by the Builders team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create an Actor and change a version's files over MCP (create-actor, update-actor-version)

5 participants