Repository navigation
Conversation
…of mutating the shared one
…as the Docker context
…to feat/push-validate-actor-json-paths
…ths' into feat/push-monorepo-upload
…to feat/push-validate-actor-json-paths
…ths' into feat/push-monorepo-upload
…to feat/push-validate-actor-json-paths
…ths' into feat/push-monorepo-upload # Conflicts: # src/lib/docker-context.ts
push reads through a symlink and uploads the content at the link's own path, so a link to a file outside the upload root builds on the platform. The realpath escape rule refused it; the uploaded files rule already rejects anything push does not send.
The worker copies inputSchema over input and outputSchema over output before resolving, so a schema path under the alias was never checked.
The worker treats an empty string as an unset field, so an empty value was wrongly reported as the .actor folder being a directory. A backslash passes path.resolve on Windows but is a filename character on the Linux worker, so it is refused before any resolution.
The lib test now builds its fixture through useTempPath, as the other lib tests do, adds a changelog file so the case-insensitive match is checked for all three listed fields, and covers the reason given when the uploaded file differs only in case. The command test restores the real process.exitCode next to the mocked copy.
checkActorPaths resolves the roots and builds the posix set of uploaded files once and hands them to every field, instead of each field recomputing them.
…r case The upload root comes from the native realpath, with names in their case on disk, but the Actor folder's files were checked against it with the JS realpath, which keeps the case of the link's text. On a case-insensitive file system such a link inside the repository was dropped without a message.
After a case-only rename without git mv on a case-insensitive file system, the index keeps the old case, so the clone list kept the folder's tracked files next to the ordinary-push list under actorPath. That doubled the Actor and bypassed its .actorignore for tracked files. The folder is now matched without regard to case or normalization and confirmed on disk.
|
Don't merge yet. This PR depends on:
Until then, CI passes, but monorepo pushes don't work. Without the core change, the API rejects |
…upload # Conflicts: # src/lib/utils.ts # test/local/commands/push-monorepo.test.ts
…th a space included Past 1 MiB of names, git's output hit execFileSync's default buffer, and the push silently fell back to the non-Git file list. Trimming the output also cut the space off a first name that starts with one.
…as its root The worker roots a zip with no file at its top level at its last folder, as it does a GitHub archive, and resolves actorPath from there. An upload root with only folders at its top level then failed the build. Ordinary zips keep their layout.
…push uploads anything The platform resolves the path from .actor/ on Linux, so such a value can't build there. The push uploaded the whole repository first, then the build failed.
… upload Outside Git, the file list follows folder links, and only the links themselves were checked, so the files under a link to an outside folder were uploaded. Each folder's real path is now checked once.
Git state reading stopped at the missing HEAD, so the warning never showed, though every file a monorepo push uploads is uncommitted there.
A committed link to a gitignored file, such as a workspace package's .env pointing at the root .env, uploaded the file's content, and a link into .git uploaded the repository's config. A file link now goes only when its target is in the file list, as folder links already did.
apify/apify-worker#2249 now resolves actorPath from the archive's top level whenever the path is there, and this push ships after it. The source/ folder that worked around the worker's guess of the root goes, and createActZip takes no root folder again.
…der adds Links outside the Actor folder resolve as a clone of the repository would, so one to a file the clone lacks is left out. A file the Actor folder's upload adds, such as a gitignored dist that its .actorignore takes back, now counts as held too. It is matched by the target's native real path, as the Actor folder's files have their names as stored on disk, so nothing else changes: links elsewhere still resolve against the clone list as before, which tests now pin for a composed link to a decomposed Actor folder and for one in the folder's old case after a case-only rename.
…upload # Conflicts: # test/local/lib/utils.test.ts
- Give the monorepo zip a key per actorPath, so a failed version update after a rename leaves the record the version builds from intact. - Zip instead of sending multiple files when a name is over 255 characters, or when the content as sent (base64, emoji counted as five bytes) is over the multi-file limit. - Copy files through directory links inside linked directories, with a cycle guard, and through directory links to files only the Actor folder adds, under Git's spelling of the Actor folder. - Send the files next to the Actor folder under actorPath's spelling, so a case-only rename or a decomposed name doesn't split a folder in two on the platform; a symlink named like such a folder stays apart.
…e help - Remove the guard that turned a 400 for an unknown actorPath key into the old refusal (exit 255) and deleted a just-created Actor. The platform accepts actorPath now, so the guard can't trigger; a platform error now surfaces as for any other push. The refusal message and its tracking-issue URL go with it. - Replace the guard's tests with one that creates a new Actor and checks actorPath on its inline version and on the version update. - The help text said .gitignore and .actorignore exclude files, which is wrong outside the Actor folder of a monorepo push. It now says what goes: the Actor folder as before, and everything else that Git tracks or doesn't ignore, without .actorignore; outside Git, the whole Docker context. docs/ regenerated.
The tests skip the system Git config, where a Windows runner enables core.longpaths, so Git skipped the folder over 260 characters and the file never reached the upload.
l2ysho
left a comment
There was a problem hiding this comment.
I found one problem with claude if no git initialized. Personally I always start building actors with no git, but this may not fit all CLI users. Anyway it is easy to reproduce and fix. More info in claude report in comment.
| const actorFolderFiles = filePathsToPush.map((file) => `${actorPath}/${file}`); | ||
| const contextFiles = resolveLinksForUpload( | ||
| uploadRoot, | ||
| await getContextFilePaths(uploadRoot), |
There was a problem hiding this comment.
Outside Git, this uploads another Actor's local run state. actors/<other-actor>/storage/key_value_stores/default/INPUT.json lands in the pushed Actor's source files.
Repro
- Download
apify/actor-monorepo-exampleas a ZIP, so there is no.git, and unzip it. - Create the file a local run leaves behind:
mkdir -p actors/javascript-actor/storage/key_value_stores/default echo '{"message":"x"}' > actors/javascript-actor/storage/key_value_stores/default/INPUT.json
apify pushthe other Actor,actors/typescript-actor.- Its Source tab contains
actors/javascript-actor/storage/key_value_stores/default/INPUT.json.
Why
This line passes the monorepo root. getContextFilePaths filters UNTRACKED_IGNORED_SEGMENTS per path segment on its Git branch, but its non-Git fallback at src/lib/docker-context.ts:213 hands off to getActorLocalFilePaths, whose ACTOR_FILES_HARDCODED_IGNORE is a list of bare names. tinyglobby anchors those at the walk root. Every caller so far passed the Actor folder, where storage/ is at the root. Here the root is one or more levels higher, so only a top-level storage/ is ignored.
actors/typescript-actor/storage/ is fine, withActorFolderFiles drops it. node_modules/ is fine too, the example repo's root .gitignore covers it. What leaks is storage/, apify_storage/ and crawlee_storage/ below the upload root, and .gitignore in that repo names none of them.
Fix
Goes in src/lib/docker-context.ts:213, which is outside this hunk, so it can't be a suggestion here:
if (!tracked || !untracked) {
return (await getActorLocalFilePaths(root)).filter(
(file) => !file.split('/').some((segment) => UNTRACKED_IGNORED_SEGMENTS.has(segment)),
);
}The no-Git case in test/local/lib/docker-context-files.test.ts asserts ${ACTOR_PATH}/storage/ is absent, which is the Actor's own folder and a different code path. A sibling package with storage/ and apify_storage/ would catch this.
A rebase picks up #1551, which made the same fix for .git.
🤖 Generated with Claude Code
There was a problem hiding this comment.
Thanks, reproduced and fixed in 1a8ecd1. Outside Git, the fallback list now drops node_modules and local run state at any depth, as the Git branch does for untracked files. I narrowed the rule on both branches to what a local run writes (<storage folder>/<datasets|key_value_stores|request_queues>/<store>/<file>), because dropping every folder named storage would also drop source code such as packages/db/src/storage/kv.ts. Tests: a sibling package with those folders in docker-context-files.test.ts, and your repro (another Actor's storage/.../INPUT.json) in push-monorepo.test.ts. Master is merged in, so #1551 is included too.
…de Git Outside Git, the upload list fell back to the ordinary file list, which drops node_modules and the storage folders only at its root. In a monorepo that root is the repository, so another Actor's storage/key_value_stores/default/INPUT.json was pushed too. Both branches now drop node_modules and what a local run stores (<storage folder>/<datasets|key_value_stores|request_queues>/<store>/ <file>) at any depth: every file outside Git, untracked files in Git. Other files in a folder named storage stay, so source code such as src/storage/kv.ts goes, where the Git branch used to drop it when untracked.
Summary
apify pushof a monorepo Actor (one whosedockerContextDirlies outside its folder) now uploads the Git repository root, or the Docker context root outside Git, and tells the platform where the Actor lies in it through theactorPathversion field, instead of refusing with exit code 255. Both Actors of apify/actor-monorepo-example can then build from a push, as they do from a Git source.Closes #1525, part of the monorepo Epic apify/apify-core#28685. Needs apify/apify-core#31479 and apify/apify-worker#2249 deployed first (see Merge gate).
Builds on the preflight (#1502), the
actor.jsonpath checks (#1503), multi-Actor selection (#1504), the ignore rules that the Actor folder's file list uses (#1537), and howcreateActZipwrites the zip for every push (#1549), all onmasternow.Merge gate
Merge only after both platform PRs are deployed:
actorPathonSOURCE_FILESandTARBALLversions. Before it, the API rejects the key with a 400, so every monorepo push fails.actorPath. Before it, the worker ignores the field and builds from the upload root, which is the repository root, so the build can't find the Actor or builds the wrong image, after the push has already replaced the version's source.Changes
sourceRootand the file list comes fromgetContextFilePaths(Git clone semantics,/separators); outside Git it is the Docker context root with the ordinary list. Both dropnode_modulesand what local runs store (<storage folder>/<datasets|key_value_stores|request_queues>/<store>/<file>, understorage,apify_storage, orcrawlee_storage) at any depth: untracked files in Git, every file outside Git. The ordinary list drops those only at its root, which here lies above the Actor folders, so another Actor's local run state would go too. Other files in a folder namedstorage, such as source code, stay. Git's output is read whole, past the default 1 MiB buffer, so a large repository never falls back to the non-Git list, and a file name starting with a space keeps the space. The Actor folder itself goes as an ordinary push of it would send it, so its.gitignoreand.actorignoreapply asapify push --helpsays, negations such as!dist/included.--all, the picker, and--dirchoose the Actors, and each one is pushed on its own: a monorepo Actor as the repository root with its ownactorPath, any other Actor as its folder, as before.--dir, as feat: letapify pushselect which Actors to push in a repository with several #1504 decided. A clone has at most the folder's tracked files, so the rest join the upload with the Actor folder, and the platform finds the Actor atactorPath. Which files go depends on fix: leave out every gitignored file when apify push collects files #1537. Without it, an ignored folder goes with every file in it, including a.envthat its own.gitignorelists, and one that holds a tracked file loses its untracked files. An ordinary push of such a folder onmasterdoes the same today.actorPathcarries the Actor folder's names as stored on disk. A--dirtyped in another letter case on macOS or Windows still gives the right path, and the Actor folder doesn't end up in the upload twice when Git lists it in another form: composed (NFC) while it is stored decomposed (NFD), or in its old case after a case-only rename. Other files in a folder on the Actor folder's way, such aspackages/libnext topackages/actor, go under the same spelling asactorPath, so the platform doesn't write that folder twice. A link named like such a folder in another case, on a case-sensitive file system, stays a folder of its own. AdockerContextDirthat leaves the Actor folder through a symlink leading back to it exits 5 instead of sending an emptyactorPath, outside Git or when the Actor folder is the repository root.actor.jsonpath check runs with that upload root and the Docker context, so adockerfileoutside the context exits 5 before any platform call.dockerContextDir, or one with a backslash, exits 5 before any upload, as such paths to files inactor.jsondo: the platform resolves it from.actor/on Linux. Onmaster, such a value inside the Actor folder pushed, then failed the build.MAX_MULTIFILE_BYTESthe version getsSOURCE_FILESwith names relative to the upload root; otherwise the existing zip upload to theactor-<id>-sourcestore andTARBALLwith the (signed)tarballUrl. Push also zips, for every push, when a file name is over the platform's 255-character limit, which a deep path from the repository root can reach, or when the content as sent is over the limit although the files on disk aren't: base64 makes a binary file a third larger, and the platform counts each character beyond the Basic Multilingual Plane, such as an emoji, as five bytes. Onmaster, both cases fail the version update with a 400. The monorepo zip has the repository layout from its top level, where apify/apify-worker#2249 resolvesactorPath. The monorepo zip goes under a key of its own for eachactorPath,version-<version>-monorepo-<first 16 hex digits of the SHA-256 of actorPath>.zip, so a version update that fails afterwards leaves alone the record an existing version builds from, whether that version has noactorPathor another one, such as the Actor folder's name before a rename. Every version create or update payload of such a push, including the inline version of a new Actor, carriesactorPath. Ordinary pushes send no such key, and the platform clears a storedactorPathon any update that sendssourceType,sourceFiles, ortarballUrlwithout it, so a later push of the Actor folder resets an old path.os.tmpdir()for every push that zips, nottemp_file.zipin the working directory, and is removed in afinally..envlinking to the gitignored root.env, or a file in.git), or one pointing outside the repository is left out. A link to a file that only the Actor folder's upload adds, such as its gitignoreddist/that its.actorignoretakes back, goes too, and so does a directory link to such files, under the Actor folder's spelling in Git when Git spells it differently from the disk. Outside Git, where the file list follows folder links, so are the files under a link to a folder outside the upload. Theactor.jsonpath check accepts a reference to a link that is uploaded and rejects one to a link that is left out. Before this, a tracked directory symlink crashed the push with a rawEISDIRorENOENTerror.Tests
test/local/commands/push-monorepo.test.ts(mocked client, no network):actorPath, the--jsonenvelope, the zip path (store recordversion-0.0-monorepo-<hash>.zipwithapplication/zip,TARBALLwith a signedtarballUrl, notemp_file.zipleft in the working directory), a separate record after a rename whose version update fails, the zip's repository layout from its top level, the zip for a file name over 255 characters, for binary content over the limit in base64, and for emoji the platform counts over the limit, no zip left in the OS temp directory when the store call fails, and the uncommitted-changes warning.actorPathon the inline version of the Actor create and on the version update..actorignore, the Actor's ownnode_modulesandstorageleft out outside Git, another Actor's local run state left out outside Git while source code in a folder namedstoragegoes, the Actor folder's.actorignorein Git (an exclusion and a!dist/negation), an ignored Actor folder named with--dir, an Actor folder name stored decomposed, an Actor folder renamed by case only, which Git lists in its old case (on case-insensitive file systems only), and the siblings of the Actor folder underactorPath's spelling after a case-only rename of its parent (on case-insensitive file systems only) and in a parent stored decomposed (on macOS only)..envand one into.gitleft out while a link to a tracked file goes, a link to a file only the Actor folder adds, links that spell the Actor folder composed while it is stored decomposed (on macOS only) or in its old case after a case-only rename (on case-insensitive file systems only), the files under a folder link leading out of the Docker context outside Git, a linked file zipped as its content, a schema reached through a link inside the repository (accepted) and through one pointing outside (exit 5), adockerfilein a Docker context reached through a link inside the repository, a link whose target names the repository in another letter case (on case-insensitive file systems only), and a--dirthat reaches the Actor through a symlink.dockerfileoutside the Docker context, a--dirtyped in another case (on case-insensitive file systems only),--allfrom the repository root over two monorepo Actors and a plain one, and the cases kept from the preflight (feat: detectdockerContextDirinapify pushand refuse monorepo Actors before any write #1502).test/local/lib/docker-context.test.ts: an absolutedockerContextDir, one with backslashes, and one that leads back to the Actor folder through a symlink, outside Git and in Git with the Actor folder at the repository root.test/local/lib/docker-context-files.test.ts: a conflicted path listed once during an unresolved merge, a file list over 1 MiB, a name starting with a space, andnode_modulesand local run state left out at any depth, in Git (untracked only) and outside Git, while source folders namedstoragestay.test/local/lib/docker-context-state.test.ts: a repository with no commits counts as having uncommitted changes.test/local/lib/docker-context-realpath.test.ts: the resolver and the path check fall back to the JSrealpathwhere the native call fails.pnpm run lint,pnpm run format,tsc --noEmit,pnpm run build, andpnpm run test:localpass, except the two[python]suites (python_support.test.ts,works-with-spaces-in-path-to-actor.test.ts), which fail the same way onmasterbecause the local Python is 3.9.test:api, which needs a platform token, and the[api]case the issue asks for (see below). Run with Node.js 26.4.0. The cases for case-sensitive file systems ran on a case-sensitive APFS disk image.Notes for reviewers
actorPathsupport, hence the merge gate.apify-clientrelease needed: the CLI spreadsactorPathinto the version payload itself..actorignoreapplies, while a.actorignoreat the repository root applies only outside Git, and there only outside the Actor folder..actorignore; outside Git, the whole Docker context.docs/is regenerated withpnpm run update-docs.realpath, which gives names in their case on disk. Where it fails, as on Windows RAM or virtual disks, the push falls back to the JS implementation, whichmasteruses.actorPathfrom the archive's top level, which that PR tests ("in a flat archive with only directories at its top level").getContextFilePathskeeps them as a Git clone does. The push sends a linked file as its content in both the multi-file and the zip upload, so it resolves them as described under Changes. A link whose target lies outside the repository, or is a file the clone list lacks, is left out too, as a clone couldn't resolve it, unless the Actor folder's upload adds that file. This holds for links outside the Actor folder; links inside it follow the ordinary push, which reads them through.[api]case intest/api/commands/push.test.tspushing a two-Actor fixture, once the platform change is deployed.🤖 Generated with Claude Code