fix(run-with-apify-token): support all config modes by JuanGalilea · Pull Request #219 · apify/apify-test-tools

JuanGalilea · 2026-10-26T10:09:11Z

As mentioned in #213, script currently takes only the default (legacy) config mode.
With it being moved to the typescript source and built from it, we can now reuse our config parsing code to get all tokens without having to care about config modes or re-writing the logic by hand.

This reuses the shared config parser in run-with-apify-tokens to support both legacy and grouped configs, including group defaults and actor overrides.

Only read the config settings needed for token forwarding, without checking local paths or loading actor.json.

Empty or missing required tokenEnvVar values now fail config validation instead of producing warnings. Declared tokens without a matching secret still produce warnings.

JuanGalilea · 2026-10-29T11:01:09Z

this will conflict with #225, the conflict is minor thought since they are somewhat orthogonal

metalwarrior665

Reusing the config parser is the right fix for grouped mode. I rebuilt the bundles locally (they match what's committed) and the bundle and config tests pass.

This depends on the rebase requested on #213: once #213 ports #225, this PR's test needs WORKFLOW_SECRETS (inline). The other comments are small.

FYI, the generated script grows from about 3 KB to 467 KB, almost all of it zod. It's marked generated and the size doesn't matter at runtime, so no change needed.

Generated by Claude Code

metalwarrior665 · 2026-11-03T16:13:40Z

+    actorConfigs = await readBaseConfigFile();
 } catch (error) {
-    fail(`Cannot read "${configPath}": ${error instanceof Error ? error.message : String(error)}`);
+    fail(`Cannot read "${CONFIG_FILE_NAME}": ${error instanceof Error ? error.message : String(error)}`);


This prefix is now wrong in most cases, and it repeats the message underneath:

Missing file: Cannot read "apify-test-tools.config.json": Config file "apify-test-tools.config.json" not found in the current directory…

Schema error: Cannot read "apify-test-tools.config.json": ✖ Invalid string: must match pattern …, even though the file was read fine.

readConfigFileContents and the parsers already write messages for the user, so passing error.message through, or using a neutral prefix like Invalid config:, would read better.

Generated by Claude Code

solved, got rid of the whole prefix

metalwarrior665 · 2026-11-03T16:13:44Z

-            `Building those Actors will fail.`,
-    );
-}
+const tokenNames = [...new Set(actorConfigs.map(({ tokenEnvVar }) => tokenEnvVar))].sort();


The schemas accept "tokenEnvVar": "" (z.string() in legacy.ts and grouped.ts). The old script filtered empty names out, but now "" lands in tokenNames, and the run prints:

Warning: apify-test-tools.config.json declares , but no such secret was passed to this workflow. …

.min(1) on those schemas would fix it here and in the CLI. Filtering '' here is the local alternative.

Also worth a line in the description: an entry without tokenEnvVar used to be a warning (the entriesWithoutToken path). Now it fails the whole step at parse time. That's consistent with the CLI, which rejects the same config anyway, but users will notice.

Generated by Claude Code

set all parsers to enforce min length 1

metalwarrior665 · 2026-11-03T16:13:46Z


 /** Stage 1 — the config file as a plain object, or a failure phrased for whoever wrote it. */
-const readConfigFileContents = async (): Promise<Record<string, unknown>> => {
+export const readConfigFileContents = async (): Promise<Record<string, unknown>> => {


Nit: this export isn't needed. readConfigFileContents is only used inside this file (by readConfigFile and readBaseConfigFile), and check-unused will flag it once that check is turned on.

Generated by Claude Code

got rid of export

metalwarrior665 · 2026-11-03T16:13:49Z

+            }) satisfies PathlessConfig,
+        // `satisfies` so its not shown as `PathlessConfig` type on output or IDE, but the expanded form


Nit: this comment sits in .map()'s argument list, after the callback, so it reads like a second argument. Moving it above the object literal (or the return) would make it clearer.

Generated by Claude Code

moved it to right before of the return of the arrow function

metalwarrior665 · 2026-11-03T16:13:52Z

        // TODO: drop folder uniqueness (this requires several changes on other places)
        if (seenFolders.has(entry.folder.path)) {
-            errors.push(`Duplicate folder "${body[index].folder}". Each actor must have a unique folder.`);
+            errors.push(`Duplicate folder "${resolved[index].folder}". Each actor must have a unique folder.`);


resolved[index] assumes body and resolved are in the same order. That holds because initializePaths maps one-to-one, but it's not obvious from the signature. A one-line comment (or doc on the resolved parameter) would help.

Generated by Claude Code

got rid of resolved altogether since it was only being used to print the raw folder.

It could cause some confusion but i guess thats what you get for passing 2 paths that normalize to the same path. In theory it should never happen, why would you pass in notIncluded/../path/to/./file doesn't even make sense.

metalwarrior665 · 2026-11-03T16:13:54Z

+        for (const name of ['one', 'two']) {
+            await mkdir(path.join(repoDir, 'actors', name), { recursive: true });
+        }


After b23803d the script no longer checks folders, so these directories are leftover setup. Removing them, or pointing the config at folders that don't exist, would also test the "no path checks" behaviour end to end. Right now only the unit test covers it.

Generated by Claude Code

ty ty, fixed

metalwarrior665 · 2026-11-03T16:13:57Z

+            cwd: repoDir,
+            env: {
+                PATH: process.env.PATH ?? '',
+                ALL_SECRETS: JSON.stringify({


This will need to become WORKFLOW_SECRETS once #213 is rebased onto #225 (see my comment there). While you're here, a case where a token comes only from inside CONFIG_SECRETS would make sure the next rebase can't drop that path unnoticed.

Generated by Claude Code

rebased and got its changes from #213

JuanGalilea self-assigned this Oct 26, 2026

JuanGalilea added the actions label Oct 26, 2026

JuanGalilea requested review from metalwarrior665 and ruocco-l as code owners October 26, 2026 10:09

JuanGalilea changed the base branch from master to workflow-script-bundle October 26, 2026 10:09

JuanGalilea added this pull request to stack #220 October 26, 2026 10:09

metalwarrior665 reviewed Nov 3, 2026

View reviewed changes

JuanGalilea added 2 commits November 4, 2026 13:11

fix(script/run-with-apify-token): support all config files

2ef4141

skip file-related verification, work off fullName and tokens only

308350c

JuanGalilea force-pushed the workflow-script-read-config branch from b23803d to 308350c Compare November 4, 2026 12:12

JuanGalilea added 5 commits November 4, 2026 14:23

fix tests on secrets

4f1e9c2

enforce nonempty tokenEnvVar

f12862d

delete unused export, fix satisfies comment position

edad7b0

get rid of resolved on validation that forced array matching

0df5a13

remove unused folders from wrapper test

6338034

JuanGalilea requested a review from metalwarrior665 November 4, 2026 13:57

		}) satisfies PathlessConfig,
		// `satisfies` so its not shown as `PathlessConfig` type on output or IDE, but the expanded form

Conversation

JuanGalilea commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JuanGalilea commented Oct 29, 2026

Uh oh!

metalwarrior665 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JuanGalilea commented Oct 26, 2026 •

edited

Loading