Skip to content

feat: detect dockerContextDir in apify push and refuse monorepo Actors before any write - #1502

Merged
DaveHanns merged 12 commits into
masterfrom
feat/push-monorepo-preflight
Oct 27, 2026
Merged

DaveHanns merged 12 commits into
masterfrom
feat/push-monorepo-preflight

Conversation

@DaveHanns

@DaveHanns DaveHanns commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

An Actor whose .actor/actor.json sets dockerContextDir above its own folder (a monorepo Actor, as in apify/actor-monorepo-example) cannot be built from what apify push uploads, and --force replaces a working Git source with that upload. apify push now refuses such an Actor before any Actor read or write, pointing at the tracking Epic and the Git source workaround.

Closes #1524, part of the monorepo Epic apify/apify-core#28685.

Changes

  • New src/lib/docker-context.ts with resolveDockerContext, getContextFilePaths, readGitState, and monorepoUnsupportedMessage, lifted from the actor-runtime branch; the one cosmetic difference is the extracted toPosix helper.
  • resolveDockerContext reports one of four kinds: none (the field is absent, with no path work), within (the context is the Actor folder or a directory inside it), outside (a monorepo Actor, with the source root and the Actor path in it), and invalid (with the message to show).
  • apify push runs the resolver before the Actor lookup, the Git source prompt, and any write. It works from the Actor folder's real path, so a symlinked --dir resolves as the platform does. An invalid dockerContextDir (such as a missing directory or one outside the Git repository) exits 5, a monorepo Actor exits 255, and an absent field or a context at or below the Actor folder pushes as usual. Messages go to stderr, so --json keeps stdout clean; the refusal names the folder when actor.json has no name.

Tests

  • test/local/lib/docker-context.test.ts: the resolver inside and outside Git.
  • test/local/lib/docker-context-files.test.ts: the file collector and its no-Git fallback.
  • test/local/lib/docker-context-state.test.ts: readGitState.
  • test/local/commands/push-monorepo.test.ts: apify push on a monorepo-shaped repository with a mocked client.

Notes for reviewers

  • The actor-runtime branch (feat: Add local Actor runtime (apify runtime) #1496) should merge master afterward and drop its source-context.ts for this module.
  • The kinds were split at the repository owner's request so a context inside the Actor folder is named within rather than none, and such a context is now checked to exist.
  • Versus the branch: a context at or below the Actor folder is an ordinary push, because the platform accepts it; a missing context is reported as missing before its position is checked, so "does not contain the Actor's folder" appears only for existing directories outside Git; getContextFilePaths returns forward-slash paths on every platform.
  • Versus the issue checklist: inside Git, a sibling directory in the repository is a monorepo Actor with the repository root as the source (exit 255, not 5), since the platform clone holds the whole repository. The refusal ends with the Apify Console Git source hint, not the branch's runtime clause.
  • readGitState and getContextFilePaths have no caller yet; they land now so the upload with actorPath (apify push: upload monorepo Actors with the repository root and actorPath #1525) and the branch share one module.

🤖 Generated with Claude Code

@DaveHanns
DaveHanns marked this pull request as ready for review October 26, 2026 12:05
@DaveHanns
DaveHanns requested a review from l2ysho as a code owner October 26, 2026 12:05
@apify-service-account apify-service-account added the tested Temporary label used only programatically for some analytics. label Oct 26, 2026
@apify-service-account apify-service-account added the t-builders Issues owned by the Builders team. label Oct 26, 2026

@l2ysho l2ysho 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.

One suggestion, otherwise LGTM 🚀

Comment thread src/lib/docker-context.ts
}
const realContextRoot = realpathSync(contextRoot);

const contextPathInActor = relative(realActorDir, contextRoot);

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.

Suggested change
const contextPathInActor = relative(realActorDir, contextRoot);
const contextPathInActor = relative(realActorDir, realContextRoot);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'd keep this one as is, on purpose. This line decides whether push can simply upload the Actor folder, so it has to judge the folder the way the upload will look. Push follows a symlink and stores its files under the link's own name, so the platform only ever sees the path as written, not where the link points.

Example: the Actor folder has a symlink sym pointing to a folder outside it, and actor.json sets "dockerContextDir": "../sym". Push uploads those files as a normal folder sym inside the Actor, and the platform builds it fine. contextRoot points to sym inside the Actor, so the check says "inside", which matches. realContextRoot points to the folder outside, so the check would say "outside" and refuse a push that works today.

I added a comment on the line (50fb1b8) and a test that pins this case (1af9abf); the test fails with the suggested change.

@DaveHanns
DaveHanns merged commit bd0f69d into master Oct 27, 2026
24 of 26 checks passed
@DaveHanns
DaveHanns deleted the feat/push-monorepo-preflight branch October 27, 2026 15:04
Pijukatel pushed a commit that referenced this pull request Oct 29, 2026
Master's docker-context.ts (#1502) is the one implementation of
dockerContextDir resolution now; runtime/source-context.ts keeps only
the tarball and the upload to the runtime. Its 'outside' kind replaces
the branch's 'context', readGitState replaces readGitProvenance, and
the two push-monorepo test files are merged into one. Login keeps
master's OAuth flow with the branch's API base URL resolution.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ocuAJkcWwzKNChWU8WgKU
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.

apify push: refuse monorepo Actors before any write

3 participants